2012-05-10 07:50:43

by Hiroshi Doyu

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

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 <[email protected]>
---
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 <[email protected]>");
+MODULE_ALIAS("platform:tegra-smmu");
MODULE_LICENSE("GPL v2");
--
1.7.5.4


2012-05-10 20:08:40

by Stephen Warren

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

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 <[email protected]>

Acked-by: Stephen Warren <[email protected]>

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

2012-05-10 22:38:33

by Grant Likely

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

On Thu, 10 May 2012 14:08:33 -0600, Stephen Warren <[email protected]> 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 <[email protected]>
>
> Acked-by: Stephen Warren <[email protected]>

Acked-by: Grant Likely <[email protected]>

g.

2012-05-11 09:47:11

by Joerg Roedel

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

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 <[email protected]>
>
> Acked-by: Stephen Warren <[email protected]>
>
> 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

2012-05-11 18:05:06

by Stephen Warren

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

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 <[email protected]>
>>
>> Acked-by: Stephen Warren <[email protected]>
>>
>> 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.

2012-05-14 17:41:30

by Stephen Warren

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

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 <[email protected]>
> ---
> 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.

2012-05-17 16:48:46

by Stephen Warren

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

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 <[email protected]>
>> ---
>> 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.

2012-05-18 05:51:43

by Hiroshi Doyu

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

Hi Stephen and Joerg,

On Thu, 17 May 2012 18:48:39 +0200
Stephen Warren <[email protected]> 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 <[email protected]>
> >> ---
> >> 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.

2012-05-18 06:43:50

by Hiroshi Doyu

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

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 <[email protected]>
---
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

2012-05-18 15:15:43

by Stephen Warren

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

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 <[email protected]>

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 <[email protected]>

2012-05-18 20:56:36

by Hiroshi Doyu

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

Hi Stephen, Joerg,

Stephen Warren <[email protected]> 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 <[email protected]>
>
> 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.

2012-05-21 12:47:12

by Joerg Roedel

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

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