[PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Hiroshi DOYU]

The necessary info is expected to pass from DT.

For more precise resource reservation, there shouldn't be any
overlapping of register range between SMMU and MC. SMMU register
offset needs to be calculated correctly, based on its register bank.

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
This patch requires the following ones, sent earlier:

DMA window:
http://article.gmane.org/gmane.linux.ports.tegra/4603

AHB:
http://article.gmane.org/gmane.linux.ports.tegra/4657
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |   21 +++
 drivers/iommu/Kconfig                              |    2 +-
 drivers/iommu/tegra-smmu.c                         |  150 +++++++++++++-------
 3 files changed, 118 insertions(+), 55 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
new file mode 100644
index 0000000..89fb543
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -0,0 +1,21 @@
+NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit)
+
+Required properties:
+- compatible : "nvidia,tegra30-smmu"
+- reg : Should contain 3 register banks(address and length) for each
+  of the SMMU register blocks.
+- interrupts : Should contain MC General interrupt.
+- nvidia,#asids : # of ASIDs
+- dma-window : IOVA start address and length.
+- nvidia,ahb : phandle to the ahb bus connected to SMMU.
+
+Example:
+	smmu {
+		compatible = "nvidia,tegra30-smmu";
+		reg = <0x7000f010 0x02c
+		       0x7000f1f0 0x010
+		       0x7000f228 0x05c>;
+		nvidia,#asids = <4>;		/* # of ASIDs */
+		dma-window = <0 0x40000000>;	/* IOVA start & length */
+		nvidia,ahb = <&ahb>;
+	};
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c698437..bc711b3 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -154,7 +154,7 @@ config TEGRA_IOMMU_GART
 
 config TEGRA_IOMMU_SMMU
 	bool "Tegra SMMU IOMMU Support"
-	depends on ARCH_TEGRA_3x_SOC
+	depends on ARCH_TEGRA_3x_SOC && TEGRA_AHB
 	select IOMMU_API
 	help
 	  Enables support for remapping discontiguous physical memory
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index ecd6790..dbaba77 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -30,12 +30,15 @@
 #include <linux/sched.h>
 #include <linux/iommu.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 
 #include <asm/page.h>
 #include <asm/cacheflush.h>
 
 #include <mach/iomap.h>
 #include <mach/smmu.h>
+#include <mach/tegra-ahb.h>
 
 /* bitmap of the page sizes currently supported */
 #define SMMU_IOMMU_PGSIZES	(SZ_4K)
@@ -111,12 +114,6 @@
 
 #define SMMU_PDE_NEXT_SHIFT		28
 
-/* AHB Arbiter Registers */
-#define AHB_XBAR_CTRL				0xe0
-#define AHB_XBAR_CTRL_SMMU_INIT_DONE_DONE	1
-#define AHB_XBAR_CTRL_SMMU_INIT_DONE_SHIFT	17
-
-#define SMMU_NUM_ASIDS				4
 #define SMMU_TLB_FLUSH_VA_SECTION__MASK		0xffc00000
 #define SMMU_TLB_FLUSH_VA_SECTION__SHIFT	12 /* right shift */
 #define SMMU_TLB_FLUSH_VA_GROUP__MASK		0xffffc000
@@ -136,6 +133,7 @@
 
 #define SMMU_PAGE_SHIFT 12
 #define SMMU_PAGE_SIZE	(1 << SMMU_PAGE_SHIFT)
+#define SMMU_PAGE_MASK	((1 << SMMU_PAGE_SHIFT) - 1)
 
 #define SMMU_PDIR_COUNT	1024
 #define SMMU_PDIR_SIZE	(sizeof(unsigned long) * SMMU_PDIR_COUNT)
@@ -177,6 +175,8 @@
 #define SMMU_ASID_DISABLE	0
 #define SMMU_ASID_ASID(n)	((n) & ~SMMU_ASID_ENABLE(0))
 
+#define NUM_SMMU_REG_BANKS	3
+
 #define smmu_client_enable_hwgrp(c, m)	smmu_client_set_hwgrp(c, m, 1)
 #define smmu_client_disable_hwgrp(c)	smmu_client_set_hwgrp(c, 0, 0)
 #define __smmu_client_enable_hwgrp(c, m) __smmu_client_set_hwgrp(c, m, 1)
@@ -235,7 +235,7 @@ struct smmu_as {
  * Per SMMU device - IOMMU device
  */
 struct smmu_device {
-	void __iomem	*regs, *regs_ahbarb;
+	void __iomem	*regs[NUM_SMMU_REG_BANKS];
 	unsigned long	iovmm_base;	/* remappable base address */
 	unsigned long	page_count;	/* total remappable size */
 	spinlock_t	lock;
@@ -252,29 +252,47 @@ struct smmu_device {
 	unsigned long translation_enable_1;
 	unsigned long translation_enable_2;
 	unsigned long asid_security;
+
+	struct device_node *ahb;
 };
 
 static struct smmu_device *smmu_handle; /* unique for a system */
 
 /*
- *	SMMU/AHB register accessors
+ *	SMMU register accessors
  */
 static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
 {
-	return readl(smmu->regs + offs);
-}
-static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
-{
-	writel(val, smmu->regs + offs);
+	BUG_ON(offs < 0x10);
+	if (offs < 0x3c)
+		return readl(smmu->regs[0] + offs - 0x10);
+	BUG_ON(offs < 0x1f0);
+	if (offs < 0x200)
+		return readl(smmu->regs[1] + offs - 0x1f0);
+	BUG_ON(offs < 0x228);
+	if (offs < 0x284)
+		return readl(smmu->regs[2] + offs - 0x228);
+	BUG();
 }
 
-static inline u32 ahb_read(struct smmu_device *smmu, size_t offs)
-{
-	return readl(smmu->regs_ahbarb + offs);
-}
-static inline void ahb_write(struct smmu_device *smmu, u32 val, size_t offs)
+static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
 {
-	writel(val, smmu->regs_ahbarb + offs);
+	BUG_ON(offs < 0x10);
+	if (offs < 0x3c) {
+		writel(val, smmu->regs[0] + offs - 0x10);
+		return;
+	}
+	BUG_ON(offs < 0x1f0);
+	if (offs < 0x200) {
+		writel(val, smmu->regs[1] + offs - 0x1f0);
+		return;
+	}
+	BUG_ON(offs < 0x228);
+	if (offs < 0x284) {
+		writel(val, smmu->regs[2] + offs - 0x228);
+		return;
+	}
+	BUG();
 }
 
 #define VA_PAGE_TO_PA(va, page)	\
@@ -370,7 +388,7 @@ static void smmu_flush_regs(struct smmu_device *smmu, int enable)
 	FLUSH_SMMU_REGS(smmu);
 }
 
-static void smmu_setup_regs(struct smmu_device *smmu)
+static int smmu_setup_regs(struct smmu_device *smmu)
 {
 	int i;
 	u32 val;
@@ -398,10 +416,7 @@ static void smmu_setup_regs(struct smmu_device *smmu)
 
 	smmu_flush_regs(smmu, 1);
 
-	val = ahb_read(smmu, AHB_XBAR_CTRL);
-	val |= AHB_XBAR_CTRL_SMMU_INIT_DONE_DONE <<
-		AHB_XBAR_CTRL_SMMU_INIT_DONE_SHIFT;
-	ahb_write(smmu, val, AHB_XBAR_CTRL);
+	return tegra_ahb_enable_smmu(smmu->ahb);
 }
 
 static void flush_ptc_and_tlb(struct smmu_device *smmu,
@@ -873,52 +888,73 @@ static int tegra_smmu_resume(struct device *dev)
 {
 	struct smmu_device *smmu = dev_get_drvdata(dev);
 	unsigned long flags;
+	int err;
 
 	spin_lock_irqsave(&smmu->lock, flags);
-	smmu_setup_regs(smmu);
+	err = smmu_setup_regs(smmu);
 	spin_unlock_irqrestore(&smmu->lock, flags);
-	return 0;
+	return err;
 }
 
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
-	struct resource *regs, *regs2, *window;
 	struct device *dev = &pdev->dev;
-	int i, err = 0;
+	int i, asids, err = 0;
+	dma_addr_t base;
+	size_t size;
+	const void *prop;
 
 	if (smmu_handle)
 		return -EIO;
 
 	BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
 
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	regs2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	window = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	if (!regs || !regs2 || !window) {
-		dev_err(dev, "No SMMU resources\n");
-		return -ENODEV;
-	}
-
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
 	if (!smmu) {
 		dev_err(dev, "failed to allocate smmu_device\n");
 		return -ENOMEM;
 	}
 
-	smmu->dev = dev;
-	smmu->num_as = SMMU_NUM_ASIDS;
-	smmu->iovmm_base = (unsigned long)window->start;
-	smmu->page_count = resource_size(window) >> SMMU_PAGE_SHIFT;
-	smmu->regs = devm_ioremap(dev, regs->start, resource_size(regs));
-	smmu->regs_ahbarb = devm_ioremap(dev, regs2->start,
-					 resource_size(regs2));
-	if (!smmu->regs || !smmu->regs_ahbarb) {
-		dev_err(dev, "failed to remap SMMU registers\n");
-		err = -ENXIO;
-		goto fail;
+	for (i = 0; i < ARRAY_SIZE(smmu->regs); i++) {
+		struct resource *res;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!res)
+			return -ENODEV;
+		smmu->regs[i] = devm_request_and_ioremap(&pdev->dev, res);
+		if (!smmu->regs[i])
+			return -EBUSY;
 	}
 
+	err = of_get_dma_window(dev->of_node, "dma-window", 0, NULL,
+				&base, &size);
+	if (err)
+		return -ENODEV;
+
+	if (size & SMMU_PAGE_MASK)
+		return -EINVAL;
+
+	size >>= SMMU_PAGE_SHIFT;
+	if (!size)
+		return -EINVAL;
+
+	prop = of_get_property(dev->of_node, "nvidia,#asids", NULL);
+	if (!prop)
+		return -ENODEV;
+	asids = be32_to_cpup(prop);
+	if (!asids)
+		return -ENODEV;
+
+	smmu->ahb = of_parse_phandle(dev->of_node, "nvidia,ahb", 0);
+	if (!smmu->ahb)
+		return -ENODEV;
+
+	smmu->dev = dev;
+	smmu->num_as = asids;
+	smmu->iovmm_base = base;
+	smmu->page_count = size;
+
 	smmu->translation_enable_0 = ~0;
 	smmu->translation_enable_1 = ~0;
 	smmu->translation_enable_2 = ~0;
@@ -945,7 +981,9 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 		INIT_LIST_HEAD(&as->client);
 	}
 	spin_lock_init(&smmu->lock);
-	smmu_setup_regs(smmu);
+	err = smmu_setup_regs(smmu);
+	if (err)
+		goto fail;
 	platform_set_drvdata(pdev, smmu);
 
 	smmu->avp_vector_page = alloc_page(GFP_KERNEL);
@@ -958,10 +996,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 fail:
 	if (smmu->avp_vector_page)
 		__free_page(smmu->avp_vector_page);
-	if (smmu->regs)
-		devm_iounmap(dev, smmu->regs);
-	if (smmu->regs_ahbarb)
-		devm_iounmap(dev, smmu->regs_ahbarb);
 	if (smmu && smmu->as) {
 		for (i = 0; i < smmu->num_as; i++) {
 			if (smmu->as[i].pdir_page) {
@@ -993,8 +1027,6 @@ static int tegra_smmu_remove(struct platform_device *pdev)
 		__free_page(smmu->avp_vector_page);
 	if (smmu->regs)
 		devm_iounmap(dev, smmu->regs);
-	if (smmu->regs_ahbarb)
-		devm_iounmap(dev, smmu->regs_ahbarb);
 	devm_kfree(dev, smmu);
 	smmu_handle = NULL;
 	return 0;
@@ -1005,6 +1037,14 @@ const struct dev_pm_ops tegra_smmu_pm_ops = {
 	.resume		= tegra_smmu_resume,
 };
 
+#ifdef CONFIG_OF
+static struct of_device_id tegra_smmu_of_match[] __devinitdata = {
+	{ .compatible = "nvidia,tegra30-smmu", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, tegra_smmu_of_match);
+#endif
+
 static struct platform_driver tegra_smmu_driver = {
 	.probe		= tegra_smmu_probe,
 	.remove		= tegra_smmu_remove,
@@ -1012,6 +1052,7 @@ static struct platform_driver tegra_smmu_driver = {
 		.owner	= THIS_MODULE,
 		.name	= "tegra-smmu",
 		.pm	= &tegra_smmu_pm_ops,
+		.of_match_table = of_match_ptr(tegra_smmu_of_match),
 	},
 };
 
@@ -1031,4 +1072,5 @@ module_exit(tegra_smmu_exit);
 
 MODULE_DESCRIPTION("IOMMU API for SMMU in Tegra30");
 MODULE_AUTHOR("Hiroshi DOYU <hdoyu@nvidia.com>");
+MODULE_ALIAS("platform:tegra-smmu");
 MODULE_LICENSE("GPL v2");
-- 
1.7.5.4

[PATCH 2/2] ARM: dt: tegra30.dtsi: Add SMMU node

[Hiroshi DOYU]

Add a node for the Tegra30 SMMU

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra30.dtsi |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 167ccbc..e9792ac 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -238,4 +238,14 @@
 		       0x7000f284 0x17c>;
 		interrupts = <0 77 0x04>;
 	};
+
+	smmu {
+		compatible = "nvidia,tegra30-smmu";
+		reg = <0x7000f010 0x02c
+		       0x7000f1f0 0x010
+		       0x7000f228 0x05c>;
+		nvidia,#asids = <4>;		/* # of ASIDs */
+		dma-window = <0 0x40000000>;	/* IOVA start & length */
+		nvidia,ahb = <&ahb>;
+	};
 };
-- 
1.7.5.4

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Stephen Warren]

On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
> The necessary info is expected to pass from DT.
> 
> For more precise resource reservation, there shouldn't be any
> overlapping of register range between SMMU and MC. SMMU register
> offset needs to be calculated correctly, based on its register bank.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>

Acked-by: Stephen Warren <swarren@wwwdotorg.org>

I expect patch 1 will go through the IOMMU tree, and I'll take patch 2
through the Tegra tree.

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Grant Likely]

On Thu, 10 May 2012 14:08:33 -0600, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
> > The necessary info is expected to pass from DT.
> > 
> > For more precise resource reservation, there shouldn't be any
> > overlapping of register range between SMMU and MC. SMMU register
> > offset needs to be calculated correctly, based on its register bank.
> > 
> > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> 
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Acked-by: Grant Likely <grant.likely@secretlab.ca>

g.

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Joerg Roedel]

On Thu, May 10, 2012 at 02:08:33PM -0600, Stephen Warren wrote:
> On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
> > The necessary info is expected to pass from DT.
> > 
> > For more precise resource reservation, there shouldn't be any
> > overlapping of register range between SMMU and MC. SMMU register
> > offset needs to be calculated correctly, based on its register bank.
> > 
> > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> 
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> 
> I expect patch 1 will go through the IOMMU tree, and I'll take patch 2
> through the Tegra tree.

Any reason for splitting it up? The patches touch mostly drivers/iommu.
So to avoid conflicts its probably the best to apply them together.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Stephen Warren]

On 05/11/2012 03:46 AM, Joerg Roedel wrote:
> On Thu, May 10, 2012 at 02:08:33PM -0600, Stephen Warren wrote:
>> On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
>>> The necessary info is expected to pass from DT.
>>>
>>> For more precise resource reservation, there shouldn't be any
>>> overlapping of register range between SMMU and MC. SMMU register
>>> offset needs to be calculated correctly, based on its register bank.
>>>
>>> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
>>
>> Acked-by: Stephen Warren <swarren@wwwdotorg.org>
>>
>> I expect patch 1 will go through the IOMMU tree, and I'll take patch 2
>> through the Tegra tree.
> 
> Any reason for splitting it up? The patches touch mostly drivers/iommu.
> So to avoid conflicts its probably the best to apply them together.

Patch 1 is the driver that touches drivers/iommu and related
documentation, whereas patch 2 touches the device tree file in
arch/arm/boot/dts. Either of the patches would get conflicts if they
were merged into the "other" tree.

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Stephen Warren]

On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
> The necessary info is expected to pass from DT.
> 
> For more precise resource reservation, there shouldn't be any
> overlapping of register range between SMMU and MC. SMMU register
> offset needs to be calculated correctly, based on its register bank.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> ---
> This patch requires the following ones, sent earlier:
> 
> DMA window:
> http://article.gmane.org/gmane.linux.ports.tegra/4603

This patch has been merged even though the patch above isn't present in
the kernel anywhere, let alone in the history of the iommu tree. This
breaks compilation in linux-next.

> AHB:
> http://article.gmane.org/gmane.linux.ports.tegra/4657

This patch is in linux-next, but also is not in the history of the iommu
tree, so will break bisection of the iommu tree for Tegra.

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Stephen Warren]

On 05/14/2012 11:41 AM, Stephen Warren wrote:
> On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
>> The necessary info is expected to pass from DT.
>>
>> For more precise resource reservation, there shouldn't be any
>> overlapping of register range between SMMU and MC. SMMU register
>> offset needs to be calculated correctly, based on its register bank.
>>
>> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
>> ---
>> This patch requires the following ones, sent earlier:
>>
>> DMA window:
>> http://article.gmane.org/gmane.linux.ports.tegra/4603
> 
> This patch has been merged even though the patch above isn't present in
> the kernel anywhere, let alone in the history of the iommu tree. This
> breaks compilation in linux-next.
> 
>> AHB:
>> http://article.gmane.org/gmane.linux.ports.tegra/4657
> 
> This patch is in linux-next, but also is not in the history of the iommu
> tree, so will break bisection of the iommu tree for Tegra.

Joerg,

Can this patch please be dropped or reverted? It's still breaking
tegra_defconfig in next-20120517 since the dependencies I mentioned
above aren't satisfied. At this stage, it seems unlikely that the DMA
window parsing change is going to make it into 3.5, so this isn't just a
temporary issue.

Thanks.

Re: [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

[Hiroshi Doyu]

Hi Stephen and Joerg,

On Thu, 17 May 2012 18:48:39 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 05/14/2012 11:41 AM, Stephen Warren wrote:
> > On 05/10/2012 01:50 AM, Hiroshi DOYU wrote:
> >> The necessary info is expected to pass from DT.
> >>
> >> For more precise resource reservation, there shouldn't be any
> >> overlapping of register range between SMMU and MC. SMMU register
> >> offset needs to be calculated correctly, based on its register bank.
> >>
> >> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> >> ---
> >> This patch requires the following ones, sent earlier:
> >>
> >> DMA window:
> >> http://article.gmane.org/gmane.linux.ports.tegra/4603
> > 
> > This patch has been merged even though the patch above isn't present in
> > the kernel anywhere, let alone in the history of the iommu tree. This
> > breaks compilation in linux-next.
> > 
> >> AHB:
> >> http://article.gmane.org/gmane.linux.ports.tegra/4657
> > 
> > This patch is in linux-next, but also is not in the history of the iommu
> > tree, so will break bisection of the iommu tree for Tegra.
> 
> Joerg,
> 
> Can this patch please be dropped or reverted? It's still breaking
> tegra_defconfig in next-20120517 since the dependencies I mentioned
> above aren't satisfied. At this stage, it seems unlikely that the DMA
> window parsing change is going to make it into 3.5, so this isn't just a
> temporary issue.

I can move of_get_dma_window() function into tegra-smmu.c as a static
function. I just want SMMU available with DT for the furthuer
development. Then, it's also possible to promote that function global
later as is in the origianl patch.

I'm posting that patch follows this email. Please take it if this
proposal is acceptable.

[PATCH 1/1] iommu/tegra: smmu: Add DMA window parser

[Hiroshi DOYU]

This code was based on:
    "arch/microblaze/kernel/prom_parse.c"
    "arch/powerpc/kernel/prom_parse.c"

Can be promoted as a global function for general use.

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
Based on the discussion:
http://marc.info/?l=linux-tegra&m=133732046606458&w=2
---
 drivers/iommu/tegra-smmu.c |   50 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 192fc4a..7fc444b 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -874,6 +874,56 @@ static struct iommu_ops smmu_iommu_ops = {
 	.pgsize_bitmap	= SMMU_IOMMU_PGSIZES,
 };
 
+static int of_get_dma_window(struct device_node *dn,
+			const char *propname, int index,
+			unsigned long *busno,
+			dma_addr_t *addr, size_t *size)
+{
+	const __be32 *dma_window, *end;
+	int bytes, cur_index = 0;
+
+	if (!dn || !addr || !size)
+		return -EINVAL;
+
+	if (!propname)
+		propname = "dma-window";
+
+	dma_window = of_get_property(dn, propname, &bytes);
+	if (!dma_window)
+		return -ENODEV;
+	end = dma_window + bytes / sizeof(*dma_window);
+
+	while (dma_window < end) {
+		u32 cells;
+		const void *prop;
+
+		/* busno is always one cell */
+		if (busno)
+			*busno = be32_to_cpup(dma_window++);
+
+		prop = of_get_property(dn, "#dma-address-cells", NULL);
+		if (!prop)
+			prop = of_get_property(dn, "#address-cells", NULL);
+
+		cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
+		if (!cells)
+			return -EINVAL;
+		*addr = of_read_number(dma_window, cells);
+		dma_window += cells;
+
+		prop = of_get_property(dn, "#dma-size-cells", NULL);
+		cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
+		if (!cells)
+			return -EINVAL;
+		*size = of_read_number(dma_window, cells);
+		dma_window += cells;
+
+		if (cur_index++ == index)
+			break;
+	}
+	return 0;
+}
+
 static int tegra_smmu_suspend(struct device *dev)
 {
 	struct smmu_device *smmu = dev_get_drvdata(dev);
-- 
1.7.5.4

Re: [PATCH 1/1] iommu/tegra: smmu: Add DMA window parser

[Stephen Warren]

On 05/18/2012 12:43 AM, Hiroshi DOYU wrote:
> This code was based on:
>     "arch/microblaze/kernel/prom_parse.c"
>     "arch/powerpc/kernel/prom_parse.c"
> 
> Can be promoted as a global function for general use.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>

Hiroshi,

Once this is accepted, it might be worth following up on the original
of_get_dma_window thread and noting that once it's approved, an updated
version of the patch will be needed to remove this code at the same time.

A couple comments below though:

> +static int of_get_dma_window(struct device_node *dn,
> +			const char *propname, int index,
> +			unsigned long *busno,
> +			dma_addr_t *addr, size_t *size)
> +{
> +	const __be32 *dma_window, *end;
> +	int bytes, cur_index = 0;

Since bytes is an out parameter from of_get_property, do you need to
wrap the declaration in uninitialized_var() to prevent a compile warning
like the other patch you sent me downstream?

> +	if (!dn || !addr || !size)
> +		return -EINVAL;
> +
> +	if (!propname)
> +		propname = "dma-window";
> +
> +	dma_window = of_get_property(dn, propname, &bytes);
> +	if (!dma_window)
> +		return -ENODEV;
> +	end = dma_window + bytes / sizeof(*dma_window);
> +
> +	while (dma_window < end) {
> +		u32 cells;
> +		const void *prop;
> +
> +		/* busno is always one cell */
> +		if (busno)
> +			*busno = be32_to_cpup(dma_window++);

Shouldn't the ++ happen even if (!busno)?

Once those are fixed, in the interests of fixing the compile error,

Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Re: [PATCH 1/1] iommu/tegra: smmu: Add DMA window parser

[Hiroshi Doyu]

Hi Stephen, Joerg,

Stephen Warren <swarren@wwwdotorg.org> wrote @ Fri, 18 May 2012 17:15:34 +0200:

> On 05/18/2012 12:43 AM, Hiroshi DOYU wrote:
> > This code was based on:
> >     "arch/microblaze/kernel/prom_parse.c"
> >     "arch/powerpc/kernel/prom_parse.c"
> > 
> > Can be promoted as a global function for general use.
> > 
> > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> 
> Hiroshi,
> 
> Once this is accepted, it might be worth following up on the original
> of_get_dma_window thread and noting that once it's approved, an updated
> version of the patch will be needed to remove this code at the same time.
> 
> A couple comments below though:
> 
> > +static int of_get_dma_window(struct device_node *dn,
> > +			const char *propname, int index,
> > +			unsigned long *busno,
> > +			dma_addr_t *addr, size_t *size)
> > +{
> > +	const __be32 *dma_window, *end;
> > +	int bytes, cur_index = 0;
> 
> Since bytes is an out parameter from of_get_property, do you need to
> wrap the declaration in uninitialized_var() to prevent a compile warning
> like the other patch you sent me downstream?
> 
> > +	if (!dn || !addr || !size)
> > +		return -EINVAL;
> > +
> > +	if (!propname)
> > +		propname = "dma-window";
> > +
> > +	dma_window = of_get_property(dn, propname, &bytes);
> > +	if (!dma_window)
> > +		return -ENODEV;
> > +	end = dma_window + bytes / sizeof(*dma_window);
> > +
> > +	while (dma_window < end) {
> > +		u32 cells;
> > +		const void *prop;
> > +
> > +		/* busno is always one cell */
> > +		if (busno)
> > +			*busno = be32_to_cpup(dma_window++);
> 
> Shouldn't the ++ happen even if (!busno)?

Hm...the above code happens to work with our "dma-window" format right
now, but most likely this seems incmpatible with "ibm,dma-window"
format. I couldn't find the official format previously. I'm wondering
that those "(ibm,)dma-window" and "(ibm,)dma-range" may be able to be
generally used since those parameters are quite common to any
IOMMUs. I'll hold off those patches at least this time

Joerg, as Stephen proposed originally. Could you please revert or drop
the original one this time?

  [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

PS: I'm being off-line'd mostly next four weeks, and my response would
be delayed.

Re: [PATCH 1/1] iommu/tegra: smmu: Add DMA window parser

[joerg.roedel]

On Fri, May 18, 2012 at 10:56:19PM +0200, Hiroshi Doyu wrote:
> Joerg, as Stephen proposed originally. Could you please revert or drop
> the original one this time?
> 
>   [PATCH 1/2] iommu/tegra: smmu: Add device tree support for SMMU

I removed the patch from my tree.

Regards,

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 1/5] iommu/tegra: smmu: Add DMA window parser, of_get_dma_window()

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 20 Jun 2012 19:11:51 +0200:

> On 06/20/2012 01:16 AM, Hiroshi DOYU wrote:
> > From: Hiroshi Doyu <hdoyu@nvidia.com>
> > 
> > This code was based on:
> >     "arch/microblaze/kernel/prom_parse.c"
> >     "arch/powerpc/kernel/prom_parse.c"
> > 
> > Can be promoted as a global function for general use to replace
> > "of_parse_dma_window()" in the above. This supports different formats
> > flexibly. "prefix" can be configured if any. "busno" and "index" are
> > optionally specified. Set NULL and 0 if not used.
> > 
> > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> > ---
> > Based on the discussion:
> > http://marc.info/?l=linux-tegra&m=133732046606458&w=2
> 
> Hmmm. This function really should be in some common location and
> available for all drivers to use. Can't we add it to that common
> location from the start? What prevented the earlier patch that did this
> from getting merged into 3.5?

It's because there was no feedback against the original patches(the
common location ones, *1,*2) from DT side.

*1: http://article.gmane.org/gmane.linux.ports.tegra/4468
*2: https://lkml.org/lkml/2012/4/30/153

> One thing that might help here would be /not/ to add the common code to
> drivers/of/of_dma.c as was done in the earlier revisions of this patch -
> I believe that Grant has been trying to push subsystem-specific OF
> functionality into files in those individual subsystems, so that
> drivers/of can be kept for core support. Perhaps this patch should
> create drivers/iommu/of_iommu.c or similar?

"drivers/iommu/of_iommu.c" seems quite reasonable for this function.

> But I wonder: Is this function likely to be useful outside of
> drivers/iommu/ - you mentioned that similar code already exists in the
> two arch-specific prom_parse.c files; where are the existing users of
> those functions. If not in drivers/iommu/, then probably drivers/iommu/
> isn't a good place to put the new common function...

There are the following 3 users of of_parse_dma_window() as below. All
of them are IOMMU related, but they are architecture specific ones,
not for the standard IOMMU API. I guess that the current trend is to
convert Arch specific IOMMU API to the standard one basically.

  arch/powerpc/kernel/vio.c 	of_parse_dma_window(dev->dev.of_node, dma_window,
  arch/powerpc/platforms/cell/iommu.c 	of_parse_dma_window(np, dma_window, &index, base, size);
  arch/powerpc/platforms/pseries/iommu.c 	of_parse_dma_window(dn, dma_window, &tbl->it_index, &offset, &size);

I think that the common "dma-window" DT parser is necessary for the
standard IOMMU because "dma-window" info is dealt as
DOMAIN_ATTR_GEOMETRY in the following Joerg's patch too.

  [PATCH 0/5] IOMMU: Make IOMMU-API ready for GART-like hardware
    https://lkml.org/lkml/2012/1/19/170

If it's ok to have of_get_dma_window() in "drivers/iommu/of_iommu.c",
I'll post that version.

Any comment?

Re: [PATCH 1/5] iommu/tegra: smmu: Add DMA window parser, of_get_dma_window()

[Stephen Warren]

On 06/21/2012 12:46 AM, Hiroshi Doyu wrote:
...
> There are the following 3 users of of_parse_dma_window() as below. All
> of them are IOMMU related, but they are architecture specific ones,
> not for the standard IOMMU API. I guess that the current trend is to
> convert Arch specific IOMMU API to the standard one basically.
> 
>   arch/powerpc/kernel/vio.c 	of_parse_dma_window(dev->dev.of_node, dma_window,
>   arch/powerpc/platforms/cell/iommu.c 	of_parse_dma_window(np, dma_window, &index, base, size);
>   arch/powerpc/platforms/pseries/iommu.c 	of_parse_dma_window(dn, dma_window, &tbl->it_index, &offset, &size);
> 
> I think that the common "dma-window" DT parser is necessary for the
> standard IOMMU because "dma-window" info is dealt as
> DOMAIN_ATTR_GEOMETRY in the following Joerg's patch too.
> 
>   [PATCH 0/5] IOMMU: Make IOMMU-API ready for GART-like hardware
>     https://lkml.org/lkml/2012/1/19/170
> 
> If it's ok to have of_get_dma_window() in "drivers/iommu/of_iommu.c",
> I'll post that version.

Sounds sane to me.

[v2 1/5] iommu: Add DMA window parser, of_get_dma_window()

[Hiroshi Doyu]

From: Hiroshi DOYU <hdoyu@nvidia.com>

This code was based on:
    "arch/microblaze/kernel/prom_parse.c"
    "arch/powerpc/kernel/prom_parse.c"

Can replace "of_parse_dma_window()" in the above. This supports
different formats flexibly. "prefix" can be configured if any. "busno"
and "index" are optionally specified. Set NULL and 0 if not used.

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
Update:
Move of_get_dma_window() in drivers/iommu/of_iommu.c

Based on the discussion:
http://marc.info/?l=linux-tegra&m=133732046606458&w=2
---
 drivers/iommu/Kconfig    |    4 ++
 drivers/iommu/Makefile   |    1 +
 drivers/iommu/of_iommu.c |   90 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of_iommu.h |   21 +++++++++++
 4 files changed, 116 insertions(+), 0 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 3408937..4826af6 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -13,6 +13,10 @@ menuconfig IOMMU_SUPPORT
 
 if IOMMU_SUPPORT
 
+config OF_IOMMU
+       def_bool y
+       depends on OF
+
 # MSM IOMMU support
 config MSM_IOMMU
 	bool "MSM IOMMU Support"
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 76e54ef..14a4d5f 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_IOMMU_API) += iommu.o
+obj-$(CONFIG_OF_IOMMU)	+= of_iommu.o
 obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o msm_iommu_dev.o
 obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
 obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
new file mode 100644
index 0000000..ee249bc
--- /dev/null
+++ b/drivers/iommu/of_iommu.c
@@ -0,0 +1,90 @@
+/*
+ * OF helpers for IOMMU
+ *
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/export.h>
+#include <linux/limits.h>
+#include <linux/of.h>
+
+/**
+ * of_get_dma_window - Parse *dma-window property and returns 0 if found.
+ *
+ * @dn: device node
+ * @prefix: prefix for property name if any
+ * @index: index to start to parse
+ * @busno: Returns busno if supported. Otherwise pass NULL
+ * @addr: Returns address that DMA starts
+ * @size: Returns the range that DMA can handle
+ *
+ * This supports different formats flexibly. "prefix" can be
+ * configured if any. "busno" and "index" are optionally
+ * specified. Set 0(or NULL) if not used.
+ */
+int of_get_dma_window(struct device_node *dn, const char *prefix, int index,
+		      unsigned long *busno, dma_addr_t *addr, size_t *size)
+{
+	const __be32 *dma_window, *end;
+	int bytes, cur_index = 0;
+	char propname[NAME_MAX], addrname[NAME_MAX], sizename[NAME_MAX];
+
+	if (!dn || !addr || !size)
+		return -EINVAL;
+
+	if (!prefix)
+		prefix = "";
+
+	snprintf(propname, sizeof(propname), "%sdma-window", prefix);
+	snprintf(addrname, sizeof(addrname), "%s#dma-address-cells", prefix);
+	snprintf(sizename, sizeof(sizename), "%s#dma-size-cells", prefix);
+
+	dma_window = of_get_property(dn, propname, &bytes);
+	if (!dma_window)
+		return -ENODEV;
+	end = dma_window + bytes / sizeof(*dma_window);
+
+	while (dma_window < end) {
+		u32 cells;
+		const void *prop;
+
+		/* busno is one cell if supported */
+		if (busno)
+			*busno = be32_to_cpup(dma_window++);
+
+		prop = of_get_property(dn, addrname, NULL);
+		if (!prop)
+			prop = of_get_property(dn, "#address-cells", NULL);
+
+		cells = prop ? be32_to_cpup(prop) : of_n_addr_cells(dn);
+		if (!cells)
+			return -EINVAL;
+		*addr = of_read_number(dma_window, cells);
+		dma_window += cells;
+
+		prop = of_get_property(dn, sizename, NULL);
+		cells = prop ? be32_to_cpup(prop) : of_n_size_cells(dn);
+		if (!cells)
+			return -EINVAL;
+		*size = of_read_number(dma_window, cells);
+		dma_window += cells;
+
+		if (cur_index++ == index)
+			break;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_get_dma_window);
diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h
new file mode 100644
index 0000000..51a560f
--- /dev/null
+++ b/include/linux/of_iommu.h
@@ -0,0 +1,21 @@
+#ifndef __OF_IOMMU_H
+#define __OF_IOMMU_H
+
+#ifdef CONFIG_OF_IOMMU
+
+extern int of_get_dma_window(struct device_node *dn, const char *prefix,
+			     int index, unsigned long *busno, dma_addr_t *addr,
+			     size_t *size);
+
+#else
+
+static inline int of_get_dma_window(struct device_node *dn, const char *prefix,
+			    int index, unsigned long *busno, dma_addr_t *addr,
+			    size_t *size)
+{
+	return -EINVAL;
+}
+
+#endif	/* CONFIG_OF_IOMMU */
+
+#endif /* __OF_IOMMU_H */
-- 
1.7.5.4

Re: [v2 1/5] iommu: Add DMA window parser, of_get_dma_window()

[Stephen Warren]

On 06/21/2012 02:30 AM, Hiroshi Doyu wrote:
> From: Hiroshi DOYU <hdoyu@nvidia.com>
> 
> This code was based on:
>     "arch/microblaze/kernel/prom_parse.c"
>     "arch/powerpc/kernel/prom_parse.c"
> 
> Can replace "of_parse_dma_window()" in the above. This supports
> different formats flexibly. "prefix" can be configured if any. "busno"
> and "index" are optionally specified. Set NULL and 0 if not used.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>

Acked-by: Stephen Warren <swarren@wwwdotorg.org>

BTW, I only see patches 1 & 2 of 5 in this series; did you only mean to
post just 2 patches or did the others get lost on the way to my system
somehow?

Re: [v2 1/5] iommu: Add DMA window parser, of_get_dma_window()

[Hiroshi Doyu]

On Thu, 21 Jun 2012 18:18:59 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 06/21/2012 02:30 AM, Hiroshi Doyu wrote:
> > From: Hiroshi DOYU <hdoyu@nvidia.com>
> > 
> > This code was based on:
> >     "arch/microblaze/kernel/prom_parse.c"
> >     "arch/powerpc/kernel/prom_parse.c"
> > 
> > Can replace "of_parse_dma_window()" in the above. This supports
> > different formats flexibly. "prefix" can be configured if any. "busno"
> > and "index" are optionally specified. Set NULL and 0 if not used.
> > 
> > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> 
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Thanks.

> BTW, I only see patches 1 & 2 of 5 in this series; did you only mean to
> post just 2 patches or did the others get lost on the way to my system
> somehow?

I reposted only [1/5] and [2/5] as "v2" because the rests are
same. Probably it was better to send a whole series again. Sorry for
confusion.

Re: [v2 1/5] iommu: Add DMA window parser, of_get_dma_window()

[joerg.roedel]

On Mon, Jun 25, 2012 at 08:15:43AM +0300, Hiroshi Doyu wrote:
> On Thu, 21 Jun 2012 18:18:59 +0200
> Stephen Warren <swarren@wwwdotorg.org> wrote:
> 
> > On 06/21/2012 02:30 AM, Hiroshi Doyu wrote:
> > > From: Hiroshi DOYU <hdoyu@nvidia.com>
> > > 
> > > This code was based on:
> > >     "arch/microblaze/kernel/prom_parse.c"
> > >     "arch/powerpc/kernel/prom_parse.c"
> > > 
> > > Can replace "of_parse_dma_window()" in the above. This supports
> > > different formats flexibly. "prefix" can be configured if any. "busno"
> > > and "index" are optionally specified. Set NULL and 0 if not used.
> > > 
> > > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> > 
> > Acked-by: Stephen Warren <swarren@wwwdotorg.org>
> 
> Thanks.
> 
> > BTW, I only see patches 1 & 2 of 5 in this series; did you only mean to
> > post just 2 patches or did the others get lost on the way to my system
> > somehow?
> 
> I reposted only [1/5] and [2/5] as "v2" because the rests are
> same. Probably it was better to send a whole series again. Sorry for
> confusion.

Please repost the whole series with the Acks included as a new thread so
that I can easily apply them.


	Joerg


-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632