[PATCH 1/3] iommu/tegra: smmu: Reserve SMMU reg regions precisely

[PATCH 1/3] iommu/tegra: smmu: Reserve SMMU reg regions precisely

[Hiroshi DOYU]

SMMU register regions are located into 3 blocks discontiguously.

Get them and reserve each region respectively. This allows other
module to use/reserve other register regions between SMMU register
blocks.

Signed-off-by: Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/tegra-smmu.c |   44 ++++++++++++++++++++++++++++++++++++--------
 1 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index de9fafe..90bbf07 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -173,6 +173,8 @@
 #define SMMU_ASID_DISABLE	0
 #define SMMU_ASID_ASID(n)	((n) & ~SMMU_ASID_ENABLE(0))
 
+#define NUM_SMMU_REG_BLOCKS	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)
@@ -866,7 +868,7 @@ static int tegra_smmu_resume(struct device *dev)
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
-	struct resource *regs, *window;
+	struct resource *res[NUM_SMMU_REG_BLOCKS], *window;
 	struct device *dev = &pdev->dev;
 	int i, err = 0;
 
@@ -875,9 +877,25 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 
 	BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
 
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	window = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!regs || !window) {
+	for (i = 0; i < ARRAY_SIZE(res); i++) {
+		res[i] = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!res[i])
+			return -ENODEV;
+
+		res[i] = devm_request_mem_region(&pdev->dev, res[i]->start,
+						 resource_size(res[i]),
+						 dev_name(&pdev->dev));
+		if (res[i])
+			continue;
+
+		while (--i >= 0)
+			devm_release_mem_region(&pdev->dev, res[i]->start,
+						resource_size(res[i]));
+		return -EIO;
+	}
+
+	window = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (!window) {
 		dev_err(dev, "No SMMU resources\n");
 		return -ENODEV;
 	}
@@ -892,7 +910,8 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	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 = devm_ioremap(dev, res[0]->start,
+				  res[2]->end - res[0]->start + 1);
 	if (!smmu->regs) {
 		dev_err(dev, "failed to remap SMMU registers\n");
 		err = -ENXIO;
@@ -905,7 +924,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu->asid_security = 0;
 
 	smmu->as = devm_kzalloc(dev,
-			sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
+				sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
 	if (!smmu->as) {
 		dev_err(dev, "failed to allocate smmu_as\n");
 		err = -ENOMEM;
@@ -950,6 +969,9 @@ fail:
 		devm_kfree(dev, smmu->as);
 	}
 	devm_kfree(dev, smmu);
+	for (i = 0; i < ARRAY_SIZE(res); i++)
+		devm_release_mem_region(&pdev->dev, res[i]->start,
+					resource_size(res[i]));
 	return err;
 }
 
@@ -957,12 +979,11 @@ static int tegra_smmu_remove(struct platform_device *pdev)
 {
 	struct smmu_device *smmu = platform_get_drvdata(pdev);
 	struct device *dev = smmu->dev;
+	int i;
 
 	smmu_write(smmu, SMMU_CONFIG_DISABLE, SMMU_CONFIG);
 	platform_set_drvdata(pdev, NULL);
 	if (smmu->as) {
-		int i;
-
 		for (i = 0; i < smmu->num_as; i++)
 			free_pdir(&smmu->as[i]);
 		devm_kfree(dev, smmu->as);
@@ -973,6 +994,13 @@ static int tegra_smmu_remove(struct platform_device *pdev)
 		devm_iounmap(dev, smmu->regs);
 	devm_kfree(dev, smmu);
 	smmu_handle = NULL;
+	for (i = 0; i < NUM_SMMU_REG_BLOCKS; i++) {
+		struct resource *res;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		devm_release_mem_region(&pdev->dev, res->start,
+					resource_size(res));
+	}
 	return 0;
 }
 
-- 
1.7.5.4

[PATCH 1/3] iommu/tegra: smmu: Reserve SMMU reg regions precisely

[Hiroshi DOYU]

SMMU register regions are located into 3 blocks discontiguously.

Get them and reserve each region respectively. This allows other
module to use/reserve other register regions between SMMU register
blocks.

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
 drivers/iommu/tegra-smmu.c |   44 ++++++++++++++++++++++++++++++++++++--------
 1 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index de9fafe..90bbf07 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -173,6 +173,8 @@
 #define SMMU_ASID_DISABLE	0
 #define SMMU_ASID_ASID(n)	((n) & ~SMMU_ASID_ENABLE(0))
 
+#define NUM_SMMU_REG_BLOCKS	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)
@@ -866,7 +868,7 @@ static int tegra_smmu_resume(struct device *dev)
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
-	struct resource *regs, *window;
+	struct resource *res[NUM_SMMU_REG_BLOCKS], *window;
 	struct device *dev = &pdev->dev;
 	int i, err = 0;
 
@@ -875,9 +877,25 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 
 	BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
 
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	window = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	if (!regs || !window) {
+	for (i = 0; i < ARRAY_SIZE(res); i++) {
+		res[i] = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!res[i])
+			return -ENODEV;
+
+		res[i] = devm_request_mem_region(&pdev->dev, res[i]->start,
+						 resource_size(res[i]),
+						 dev_name(&pdev->dev));
+		if (res[i])
+			continue;
+
+		while (--i >= 0)
+			devm_release_mem_region(&pdev->dev, res[i]->start,
+						resource_size(res[i]));
+		return -EIO;
+	}
+
+	window = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	if (!window) {
 		dev_err(dev, "No SMMU resources\n");
 		return -ENODEV;
 	}
@@ -892,7 +910,8 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	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 = devm_ioremap(dev, res[0]->start,
+				  res[2]->end - res[0]->start + 1);
 	if (!smmu->regs) {
 		dev_err(dev, "failed to remap SMMU registers\n");
 		err = -ENXIO;
@@ -905,7 +924,7 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	smmu->asid_security = 0;
 
 	smmu->as = devm_kzalloc(dev,
-			sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
+				sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
 	if (!smmu->as) {
 		dev_err(dev, "failed to allocate smmu_as\n");
 		err = -ENOMEM;
@@ -950,6 +969,9 @@ fail:
 		devm_kfree(dev, smmu->as);
 	}
 	devm_kfree(dev, smmu);
+	for (i = 0; i < ARRAY_SIZE(res); i++)
+		devm_release_mem_region(&pdev->dev, res[i]->start,
+					resource_size(res[i]));
 	return err;
 }
 
@@ -957,12 +979,11 @@ static int tegra_smmu_remove(struct platform_device *pdev)
 {
 	struct smmu_device *smmu = platform_get_drvdata(pdev);
 	struct device *dev = smmu->dev;
+	int i;
 
 	smmu_write(smmu, SMMU_CONFIG_DISABLE, SMMU_CONFIG);
 	platform_set_drvdata(pdev, NULL);
 	if (smmu->as) {
-		int i;
-
 		for (i = 0; i < smmu->num_as; i++)
 			free_pdir(&smmu->as[i]);
 		devm_kfree(dev, smmu->as);
@@ -973,6 +994,13 @@ static int tegra_smmu_remove(struct platform_device *pdev)
 		devm_iounmap(dev, smmu->regs);
 	devm_kfree(dev, smmu);
 	smmu_handle = NULL;
+	for (i = 0; i < NUM_SMMU_REG_BLOCKS; i++) {
+		struct resource *res;
+
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		devm_release_mem_region(&pdev->dev, res->start,
+					resource_size(res));
+	}
 	return 0;
 }
 
-- 
1.7.5.4

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

[Hiroshi DOYU]

Add device tree support for Tegra30 IOMMU(SMMU).

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
 .../bindings/iommu/nvidia,tegra30-smmu.txt         |   20 ++++++++
 drivers/iommu/tegra-smmu.c                         |   47 +++++++++++++++-----
 2 files changed, 55 insertions(+), 12 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..6f942ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -0,0 +1,20 @@
+NVIDIA Tegra 30 IOMMU H/W, SMMU (System Memory Management Unit)
+
+Required properties:
+- compatible : "nvidia,tegra30-smmu"
+- reg : Should contain 3 register ranges(address and length) for each
+  of the SMMU register blocks.
+- interrupts : Should contain MC General interrupt.
+- asids	     : # of ASIDs
+- dma-window: IOVA start address and length.
+
+Example:
+	smmu: smmu@7000f000 {
+		compatible = "nvidia,tegra30-smmu";
+		reg = <0x7000f010 0x2c		/* controller registers */
+		       0x7000f1f0 0x10		/* statics */
+		       0x7000f228 0x5c>;	/* module configuration */
+		interrupts = <0 13 0x40>;
+		asids = <4>;			/* # of ASIDs */
+		dma-window = <0 0x40000000>;	/* IOVA start & length */
+	};
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 90bbf07..c2827a6 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -30,6 +30,8 @@
 #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>
@@ -112,7 +114,6 @@
 
 #define SMMU_PDE_NEXT_SHIFT		28
 
-#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
@@ -868,15 +869,33 @@ static int tegra_smmu_resume(struct device *dev)
 static int tegra_smmu_probe(struct platform_device *pdev)
 {
 	struct smmu_device *smmu;
-	struct resource *res[NUM_SMMU_REG_BLOCKS], *window;
+	struct resource *res[NUM_SMMU_REG_BLOCKS];
 	struct device *dev = &pdev->dev;
-	int i, err = 0;
+	int i, asids, err = 0;
+	dma_addr_t base;
+	size_t size;
+
+	if (!dev->of_node)
+		return -ENODEV;
 
 	if (smmu_handle)
 		return -EIO;
 
 	BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
 
+	err = of_parse_dma_window(dev->of_node, "dma-window", 0, NULL,
+				  &base, &size);
+	if (err)
+		return -ENODEV;
+
+	size >>= SMMU_PAGE_SHIFT;
+	if (!size)
+		return -ENODEV;
+
+	asids = be32_to_cpup(of_get_property(dev->of_node, "asids", NULL));
+	if (!asids)
+		return -ENODEV;
+
 	for (i = 0; i < ARRAY_SIZE(res); i++) {
 		res[i] = platform_get_resource(pdev, IORESOURCE_MEM, i);
 		if (!res[i])
@@ -894,12 +913,6 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 		return -EIO;
 	}
 
-	window = platform_get_resource(pdev, IORESOURCE_MEM, 3);
-	if (!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");
@@ -907,9 +920,9 @@ static int tegra_smmu_probe(struct platform_device *pdev)
 	}
 
 	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->num_as = asids;
+	smmu->iovmm_base = base;
+	smmu->page_count = size;
 	smmu->regs = devm_ioremap(dev, res[0]->start,
 				  res[2]->end - res[0]->start + 1);
 	if (!smmu->regs) {
@@ -1009,6 +1022,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,
@@ -1016,6 +1037,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),
 	},
 };
 
@@ -1035,4 +1057,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 3/3] arm/dts: Tegra30: Add device tree support for SMMU

[Hiroshi DOYU]

Add device tree support for Tegra30 IOMMU(SMMU).

Signed-off-by: Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 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 15200a9..f4929d5 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -224,4 +224,14 @@
 			nvidia,ahub-cif-ids = <8 8>;
 		};
 	};
+
+	smmu: smmu@7000f000 {
+		compatible = "nvidia,tegra30-smmu";
+		reg = <0x7000f010 0x2c		/* controller registers */
+		       0x7000f1f0 0x10		/* statics */
+		       0x7000f228 0x5c>;	/* module configuration */
+		interrupts = <0 13 0x40>;
+		asids = <4>;			/* # of ASIDs */
+		dma-window = <0 0x40000000>;	/* IOVA start & length */
+	};
 };
-- 
1.7.5.4

[PATCH 3/3] arm/dts: Tegra30: Add device tree support for SMMU

[Hiroshi DOYU]

Add device tree support for Tegra30 IOMMU(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 15200a9..f4929d5 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -224,4 +224,14 @@
 			nvidia,ahub-cif-ids = <8 8>;
 		};
 	};
+
+	smmu: smmu@7000f000 {
+		compatible = "nvidia,tegra30-smmu";
+		reg = <0x7000f010 0x2c		/* controller registers */
+		       0x7000f1f0 0x10		/* statics */
+		       0x7000f228 0x5c>;	/* module configuration */
+		interrupts = <0 13 0x40>;
+		asids = <4>;			/* # of ASIDs */
+		dma-window = <0 0x40000000>;	/* IOVA start & length */
+	};
 };
-- 
1.7.5.4

[PATCH 3/3] arm/dts: Tegra30: Add device tree support for SMMU

[Hiroshi DOYU]

Add device tree support for Tegra30 IOMMU(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 15200a9..f4929d5 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -224,4 +224,14 @@
 			nvidia,ahub-cif-ids = <8 8>;
 		};
 	};
+
+	smmu: smmu at 7000f000 {
+		compatible = "nvidia,tegra30-smmu";
+		reg = <0x7000f010 0x2c		/* controller registers */
+		       0x7000f1f0 0x10		/* statics */
+		       0x7000f228 0x5c>;	/* module configuration */
+		interrupts = <0 13 0x40>;
+		asids = <4>;			/* # of ASIDs */
+		dma-window = <0 0x40000000>;	/* IOVA start & length */
+	};
 };
-- 
1.7.5.4

Re: [PATCH 1/3] iommu/tegra: smmu: Reserve SMMU reg regions precisely

[Stephen Warren]

On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> SMMU register regions are located into 3 blocks discontiguously.
> 
> Get them and reserve each region respectively. This allows other
> module to use/reserve other register regions between SMMU register
> blocks.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

This patch depends on the other series you sent which adds the Tegra AHB
driver, and modifies the SMMU driver to use it. At least you need to
mention the dependencies so e.g. these patches don't get applied without
the other series.

> @@ -875,9 +877,25 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>  
>  	BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
>  
> -	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	window = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	if (!regs || !window) {
> +	for (i = 0; i < ARRAY_SIZE(res); i++) {
> +		res[i] = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		if (!res[i])
> +			return -ENODEV;
> +
> +		res[i] = devm_request_mem_region(&pdev->dev, res[i]->start,
> +						 resource_size(res[i]),
> +						 dev_name(&pdev->dev));
> +		if (res[i])
> +			continue;
> +
> +		while (--i >= 0)
> +			devm_release_mem_region(&pdev->dev, res[i]->start,
> +						resource_size(res[i]));

The whole point of using the devm_* functions is that you no longer need
to write out all the error-handling and freeing code related to those
allocations. A similar comment applies to many other parts of this patch.

> +		return -EIO;
> +	}
> +
> +	window = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> +	if (!window) {
>  		dev_err(dev, "No SMMU resources\n");
>  		return -ENODEV;
>  	}
> @@ -892,7 +910,8 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>  	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 = devm_ioremap(dev, res[0]->start,
> +				  res[2]->end - res[0]->start + 1);

So, you've retrieved 3 separate resources for the address space, but
only call ioremap once. That doesn't seem right; there should be a 1:1
relationship between the number of resources and ioremap calls. This is
only working because the SMMU driver isn't calling request_mem_region
for this whole range, and hence isn't conflicting with the
request_mem_region and ioremap callss that the AHB driver is (or rather,
should be) performing...

>  	smmu->as = devm_kzalloc(dev,
> -			sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
> +				sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);

That looks unrelated.

Re: [PATCH 1/3] iommu/tegra: smmu: Reserve SMMU reg regions precisely

[Stephen Warren]

On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> SMMU register regions are located into 3 blocks discontiguously.
> 
> Get them and reserve each region respectively. This allows other
> module to use/reserve other register regions between SMMU register
> blocks.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>

This patch depends on the other series you sent which adds the Tegra AHB
driver, and modifies the SMMU driver to use it. At least you need to
mention the dependencies so e.g. these patches don't get applied without
the other series.

> @@ -875,9 +877,25 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>  
>  	BUILD_BUG_ON(PAGE_SHIFT != SMMU_PAGE_SHIFT);
>  
> -	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	window = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	if (!regs || !window) {
> +	for (i = 0; i < ARRAY_SIZE(res); i++) {
> +		res[i] = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		if (!res[i])
> +			return -ENODEV;
> +
> +		res[i] = devm_request_mem_region(&pdev->dev, res[i]->start,
> +						 resource_size(res[i]),
> +						 dev_name(&pdev->dev));
> +		if (res[i])
> +			continue;
> +
> +		while (--i >= 0)
> +			devm_release_mem_region(&pdev->dev, res[i]->start,
> +						resource_size(res[i]));

The whole point of using the devm_* functions is that you no longer need
to write out all the error-handling and freeing code related to those
allocations. A similar comment applies to many other parts of this patch.

> +		return -EIO;
> +	}
> +
> +	window = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> +	if (!window) {
>  		dev_err(dev, "No SMMU resources\n");
>  		return -ENODEV;
>  	}
> @@ -892,7 +910,8 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>  	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 = devm_ioremap(dev, res[0]->start,
> +				  res[2]->end - res[0]->start + 1);

So, you've retrieved 3 separate resources for the address space, but
only call ioremap once. That doesn't seem right; there should be a 1:1
relationship between the number of resources and ioremap calls. This is
only working because the SMMU driver isn't calling request_mem_region
for this whole range, and hence isn't conflicting with the
request_mem_region and ioremap callss that the AHB driver is (or rather,
should be) performing...

>  	smmu->as = devm_kzalloc(dev,
> -			sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);
> +				sizeof(smmu->as[0]) * smmu->num_as, GFP_KERNEL);

That looks unrelated.

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

[Stephen Warren]

On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> Add device tree support for Tegra30 IOMMU(SMMU).

> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c

> +	err = of_parse_dma_window(dev->of_node, "dma-window", 0, NULL,
> +				  &base, &size);

This patch also depends on "dt: Add general DMA window parser" that you
sent earlier.

> +	asids = be32_to_cpup(of_get_property(dev->of_node, "asids", NULL));
> +	if (!asids)
> +		return -ENODEV;

What if of_get_property() fails?

BTW, does the # ASIDs vary? I wonder if it's worth representing it in
the device tree or not. If this driver has a chance of working
unmodified on some future chip just by updating this DT property without
any code changes, then it seems reasonable to have it in DT.

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

[Hiroshi Doyu]

From: Stephen Warren <swarren@wwwdotorg.org>
Subject: Re: [PATCH 2/3] iommu/tegra: smmu: Add device tree support for SMMU
Date: Mon, 23 Apr 2012 20:27:28 +0200
Message-ID: <4F959F10.9010908@wwwdotorg.org>

> On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> > Add device tree support for Tegra30 IOMMU(SMMU).
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> > +	err = of_parse_dma_window(dev->of_node, "dma-window", 0, NULL,
> > +				  &base, &size);
> 
> This patch also depends on "dt: Add general DMA window parser" that you
> sent earlier.
> 
> > +	asids = be32_to_cpup(of_get_property(dev->of_node, "asids", NULL));
> > +	if (!asids)
> > +		return -ENODEV;
> 
> What if of_get_property() fails?

Ok, need to be checked. I'll fix.

> BTW, does the # ASIDs vary? I wonder if it's worth representing it in
> the device tree or not. If this driver has a chance of working
> unmodified on some future chip just by updating this DT property without
> any code changes, then it seems reasonable to have it in DT.

Yes, ASID should be passed from DT.

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

[Hiroshi Doyu]

From: Stephen Warren <swarren@wwwdotorg.org>
Subject: Re: [PATCH 2/3] iommu/tegra: smmu: Add device tree support for SMMU
Date: Mon, 23 Apr 2012 20:27:28 +0200
Message-ID: <4F959F10.9010908@wwwdotorg.org>

> On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> > Add device tree support for Tegra30 IOMMU(SMMU).
> 
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
> 
> > +	err = of_parse_dma_window(dev->of_node, "dma-window", 0, NULL,
> > +				  &base, &size);
> 
> This patch also depends on "dt: Add general DMA window parser" that you
> sent earlier.
> 
> > +	asids = be32_to_cpup(of_get_property(dev->of_node, "asids", NULL));
> > +	if (!asids)
> > +		return -ENODEV;
> 
> What if of_get_property() fails?

Ok, need to be checked. I'll fix.

> BTW, does the # ASIDs vary? I wonder if it's worth representing it in
> the device tree or not. If this driver has a chance of working
> unmodified on some future chip just by updating this DT property without
> any code changes, then it seems reasonable to have it in DT.

Yes, ASID should be passed from DT.

Re: [PATCH 3/3] arm/dts: Tegra30: Add device tree support for SMMU

[Stephen Warren]

On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> Add device tree support for Tegra30 IOMMU(SMMU).

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +	smmu: smmu@7000f000 {
...
> +		interrupts = <0 13 0x40>;

This interrupt is on the 3rd IRQ controller (or GIC bank) so should be
(2*32)+13, i.e. 75 not 13.

Re: [PATCH 3/3] arm/dts: Tegra30: Add device tree support for SMMU

[Stephen Warren]

On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> Add device tree support for Tegra30 IOMMU(SMMU).

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +	smmu: smmu@7000f000 {
...
> +		interrupts = <0 13 0x40>;

This interrupt is on the 3rd IRQ controller (or GIC bank) so should be
(2*32)+13, i.e. 75 not 13.

[PATCH 3/3] arm/dts: Tegra30: Add device tree support for SMMU

[Stephen Warren]

On 04/23/2012 05:58 AM, Hiroshi DOYU wrote:
> Add device tree support for Tegra30 IOMMU(SMMU).

> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi

> +	smmu: smmu at 7000f000 {
...
> +		interrupts = <0 13 0x40>;

This interrupt is on the 3rd IRQ controller (or GIC bank) so should be
(2*32)+13, i.e. 75 not 13.