From: Stephen Warren <swarren@xxxxxxxxxxxxx>
Subject: Re: [PATCH v2] ARM: dt: tegra20: Add GART device
Date: Fri, 4 May 2012 19:24:45 +0200
Message-ID: <4FA410DD.3090105@xxxxxxxxxxxxx>
> On 05/04/2012 03:13 AM, Hiroshi Doyu wrote:
> > From: Stephen Warren <swarren@xxxxxxxxxxxxx>
> > Subject: Re: [PATCH v2] ARM: dt: tegra20: Add GART device
> > Date: Thu, 3 May 2012 20:28:02 +0200
> > Message-ID: <4FA2CE32.7010406@xxxxxxxxxxxxx>
> >
> >> On 04/16/2012 09:04 AM, Thierry Reding wrote:
> >>> This commit adds the device node required to probe NVIDIA Tegra 20 GART
> >>> hardware from the device tree.
> >>
> >>> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> >>
> >>> + gart: gart@7000f000 {
> >>> + compatible = "nvidia,tegra20-gart";
> >>> + reg = < 0x7000f000 0x00000100 /* controller registers */
> >>> + 0x58000000 0x02000000 >; /* GART aperture */
> >>> + };
> >>
> >> Thierry, Hiroshi, Olof,
> >>
> >> I have already applied this to Tegra's for inclusion in 3.5, but I'm
> >> considering dropping it. Further thought/discussions related to adding
> >> DT to the Tegra SMMU have raised an issue with a similar binding there.
> >>
> >> I'd like to hear peoples' thoughts on all this, and get a resolution,
> >> before we go ahead with any more GART/SMMU/MC patches.
> >>
> >> The Tegra20 register block at 7000f000 is not (just) the GART, but the
> >> MC (Memory Controller). This register block contains both general
> >> MC-related registers, and the GART registers. The situation is identical
> >> on Tegra30, except that the register block contains both MC and SMMU
> >> registers.
> >
> > For the furthur info, GART occupies 1 block within MC register range,
> > and SMMU occupies 3 blocks. In both Tegra{20,30}, MC driver only uses
> > MC registers, GART uses GART registers in its own block, and SMMU uses
> > SMMU registers in its own blocks *exclusively*.
> >
> > IRQ is handled in MC. They include IOMMU page access fault too, but
> > MC ISR just prints fault address and other info. IRQ handling
> > registers belong to MC, and GART/SMMU doesn't have to touch it.
> >
> > Tegra20:
> > 7000f000-7000f3ff : /mc@7000f000 1KB
> > 7000f024-7000f02f : gart
> >
> > Tegra30:
> > 7000f000-7000f3ff : /mc@7000f000 1KB
> > 7000f010-7000f03b : smmu, register block 1
> > 7000f1f0-7000f1ff : smmu, register block 2
> > 7000f228-7000f283 : smmu, register block 3
> > ....
>
> True, it appears that each register is logically part of MC or GART/SMMU
> functionality. In that case, chunks of registers can be assigned to
> specific drivers. However, the memory map is rather interleaved:
>
> Tegra20:
>
> 00c-01c: MC
> 024-034: GART
> 03c-270: MC
>
> Tegra30:
>
> 010-038: SMMU
> 050-220: MC
> 228-280: SMMU
> 29c-36c: MC
>
> Perhaps the simplest way is to represent those chunks as separate reg
> entries:
>
> mc@7000f010 {
> compatible = "nvidia,tegra30-mc";
> reg = <0x7000f010 0x1d4 0x70000228 0xd4>;
> };
>
> smmu@0x70000050 {
> compatible = "nvidia,tegra30-smmu";
> reg = <0x70000010 0x2c 0x70000228 0x5c>;
> };
>
> This could all be hidden inside e.g. smmu_read():
>
> static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
> {
> BUG_ON(offs < 0x10);
> if (offs < 0x40)
> return readl(smmu->regsa + offs - 0x10);
> BUG_ON(offs < 0x228);
> if (offs < 0x370)
> return readl(smmu->regsb + offs - 0x228);
> BUG();
> }
>
> This would optimize out, since most likely "offs" would be a #define for
> the register address.
>
> Then there would be no need for the DT nodes or devices/drivers to
> interact at all.
This may be quite simple and enough ok at the moment, instead of
trying to make some tricky connections between devices now.
For reference, attached the above patch to this mail.
> We should really talk to our HW engineers and make sure that future
> chips don't get even more interleaved register maps - either make sure
> the current layout is perpetuated exactly, or fix it to make them
> entirely separate. Can you please start an internal thread with them on
> this topic, Hiroshi?
Ok, Done.
> >> Option 2:
> >>
> >> We add an explicit child node to MC in order to instantiate the GART
> >> itself, and have the MC call of_platform_populate() to instantiate it,
> >> just like a bus driver:
> >>
> >> mc: mc@7000f000 {
> >> compatible = "nvidia,tegra20-mc";
> >> reg = <0x7000f000 0x00000100>;
> >>
> >> ranges;
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >>
> >> gart: gart {
> >> compatible = "nvidia,tegra20-gart";
> >> reg = <0x7000f000 0x00000100 /* controller registers */
> >> 0x58000000 0x02000000>; /* GART aperture */
> >> };
> >> };
> >>
> >> The question here is: If the MC and GART drivers both need to access
> >> basically the same register block, they can't both call
> >> request_mem_region() on the registers, which is rather unfortunate.
> >
> > Can "request_resource(mc_resource, gart_resource)" or
> > "insert_resource(mc_resource, gart_resource)" create the following
> > resource tree?
> >
> > Tegra20:
> > 7000f000-7000f3ff : /mc@7000f000 1KB
> > 7000f024-7000f02f : gart
> >
> > gart/smmu may be able to get the parent resources for ioremap()?
> >
> > + mc = pdev->dev.parent->of_node;
> > or
> > + res = platform_get_resource(to_platform_device(pdev->dev.parent), IORESOURCE_MEM, 0);
>
> I'd certainly rather avoid drivers reaching into other devices to get
> register windows, unless this is more explicitly managed by passing the
> data from one device to another via an API for this purpose. The code
> above feels quite fragile/dangerous.
Ok, let's start with the simplest way, and improve later with nice
framework.
From b8c18283190e83cd77285a56175325131baadf86 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <hdoyu@xxxxxxxxxx>
Date: Wed, 18 Apr 2012 15:26:39 +0300
Subject: [PATCH 1/1] iommu/tegra: smmu: Add DT support for SMMU
Add DT support for SMMU. The necessary info is expected to pass from
DT with more precise resource reservation, independet of the parent MC
regiter range.
Signed-off-by: Hiroshi DOYU <hdoyu@xxxxxxxxxx>
---
drivers/iommu/Kconfig | 1 +
drivers/iommu/tegra-smmu.c | 156 +++++++++++++++++++++++++++++---------------
2 files changed, 103 insertions(+), 54 deletions(-)
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index c698437..24308f4 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -155,6 +155,7 @@ config TEGRA_IOMMU_GART
config TEGRA_IOMMU_SMMU
bool "Tegra SMMU IOMMU Support"
depends on ARCH_TEGRA_3x_SOC
+ select 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..0a1954e 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
@@ -177,6 +174,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 +234,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 +251,57 @@ 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)
+
+static int get_reg_bank(u32 offset)
{
- writel(val, smmu->regs + offs);
+ int i;
+ const struct smmu_reg_bank {
+ u32 start;
+ size_t size;
+ } r[] = {
+ {
+ .start = 0x10,
+ .size = 0x2c,
+ },
+ {
+ .start = 0x1f0,
+ .size = 0x10,
+ },
+ {
+ .start = 0x228,
+ .size = 0x5c,
+ },
+ };
+
+ for (i = 0; i < ARRAY_SIZE(r); i++) {
+ BUG_ON(offset < r[i].start);
+ if (offset < r[i].start + r[i].size)
+ return i;
+ }
+ BUG();
+ return 0; /* Never reach here */
}
-static inline u32 ahb_read(struct smmu_device *smmu, size_t offs)
+static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
{
- return readl(smmu->regs_ahbarb + offs);
+ int i;
+ i = get_reg_bank(offs);
+ return readl(smmu->regs[i] + 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);
+ int i;
+ i = get_reg_bank(offs);
+ writel(val, smmu->regs[i] + offs);
}
#define VA_PAGE_TO_PA(va, page) \
@@ -370,7 +397,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 +425,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,59 +897,77 @@ 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;
+
+ size >>= SMMU_PAGE_SHIFT;
+ if (!size)
+ return -ENODEV;
+
+ 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(pdev->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;
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;
@@ -945,7 +987,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 +1002,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 +1033,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 +1043,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 +1058,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 +1078,5 @@ module_exit(tegra_smmu_exit);
MODULE_DESCRIPTION("IOMMU API for SMMU in Tegra30");
MODULE_AUTHOR("Hiroshi DOYU <hdoyu@xxxxxxxxxx>");
+MODULE_ALIAS("platform:tegra-smmu");
MODULE_LICENSE("GPL v2");
--
1.7.5.4
