2012-05-03 16:06:19

by Hiroshi Doyu

[permalink] [raw]
Subject: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
High-performance Bus (AHB) architecture.

The AHB Arbiter controls AHB bus master arbitration. This effectively
forms a second level of arbitration for access to the memory
controller through the AHB Slave Memory device. The AHB pre-fetch
logic can be configured to enhance performance for devices doing
sequential access. Each AHB master is assigned to either the high or
low priority bin. Both Tegra20/30 have this AHB bus.

Some of configuration param could be passed from DT too.

Signed-off-by: Hiroshi DOYU <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Arnd Bergmann <[email protected]>
---
arch/arm/mach-tegra/Kconfig | 8 ++
drivers/Makefile | 2 +-
drivers/amba/Makefile | 4 +-
drivers/amba/tegra-ahb.c | 272 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 283 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index 204d3d4..6a113a9 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -50,6 +50,14 @@ config TEGRA_PCI
depends on ARCH_TEGRA_2x_SOC
select PCI

+config TEGRA_AHB
+ bool "Enable AHB driver for NVIDIA Tegra SoCs"
+ default y
+ help
+ Adds AHB configuration functionality for NVIDIA Tegra SoCs,
+ which controls AHB bus master arbitration and some
+ perfomance parameters(priority, prefech size).
+
comment "Tegra board type"

config MACH_HARMONY
diff --git a/drivers/Makefile b/drivers/Makefile
index 5870322..d97e2e2 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_SFI) += sfi/
# PnP must come after ACPI since it will eventually need to check if acpi
# was used and do nothing if so
obj-$(CONFIG_PNP) += pnp/
-obj-$(CONFIG_ARM_AMBA) += amba/
+obj-$(CONFIG_ARM) += amba/
# Many drivers will want to use DMA so this has to be made available
# really early.
obj-$(CONFIG_DMA_ENGINE) += dma/
diff --git a/drivers/amba/Makefile b/drivers/amba/Makefile
index 40fe740..66e81c2 100644
--- a/drivers/amba/Makefile
+++ b/drivers/amba/Makefile
@@ -1,2 +1,2 @@
-obj-y += bus.o
-
+obj-$(CONFIG_ARM_AMBA) += bus.o
+obj-$(CONFIG_TEGRA_AHB) += tegra-ahb.o
diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
new file mode 100644
index 0000000..03933f3
--- /dev/null
+++ b/drivers/amba/tegra-ahb.c
@@ -0,0 +1,272 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION. All rights reserved.
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * Author:
+ * Jay Cheng <[email protected]>
+ * James Wylder <[email protected]>
+ * Benoit Goby <[email protected]>
+ * Colin Cross <[email protected]>
+ * Hiroshi DOYU <[email protected]>
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+
+#define DRV_NAME "tegra-ahb"
+
+#define AHB_ARBITRATION_DISABLE 0x00
+#define AHB_ARBITRATION_PRIORITY_CTRL 0x04
+#define AHB_PRIORITY_WEIGHT(x) (((x) & 0x7) << 29)
+#define PRIORITY_SELECT_USB BIT(6)
+#define PRIORITY_SELECT_USB2 BIT(18)
+#define PRIORITY_SELECT_USB3 BIT(17)
+
+#define AHB_GIZMO_AHB_MEM 0x0c
+#define ENB_FAST_REARBITRATE BIT(2)
+#define DONT_SPLIT_AHB_WR BIT(7)
+
+#define AHB_GIZMO_APB_DMA 0x10
+#define AHB_GIZMO_IDE 0x18
+#define AHB_GIZMO_USB 0x1c
+#define AHB_GIZMO_AHB_XBAR_BRIDGE 0x20
+#define AHB_GIZMO_CPU_AHB_BRIDGE 0x24
+#define AHB_GIZMO_COP_AHB_BRIDGE 0x28
+#define AHB_GIZMO_XBAR_APB_CTLR 0x2c
+#define AHB_GIZMO_VCP_AHB_BRIDGE 0x30
+#define AHB_GIZMO_NAND 0x3c
+#define AHB_GIZMO_SDMMC4 0x44
+#define AHB_GIZMO_XIO 0x48
+#define AHB_GIZMO_BSEV 0x60
+#define AHB_GIZMO_BSEA 0x70
+#define AHB_GIZMO_NOR 0x74
+#define AHB_GIZMO_USB2 0x78
+#define AHB_GIZMO_USB3 0x7c
+#define IMMEDIATE BIT(18)
+
+#define AHB_GIZMO_SDMMC1 0x80
+#define AHB_GIZMO_SDMMC2 0x84
+#define AHB_GIZMO_SDMMC3 0x88
+#define AHB_MEM_PREFETCH_CFG_X 0xd8
+#define AHB_ARBITRATION_XBAR_CTRL 0xdc
+#define AHB_MEM_PREFETCH_CFG3 0xe0
+#define AHB_MEM_PREFETCH_CFG4 0xe4
+#define AHB_MEM_PREFETCH_CFG1 0xec
+#define AHB_MEM_PREFETCH_CFG2 0xf0
+#define PREFETCH_ENB BIT(31)
+#define MST_ID(x) (((x) & 0x1f) << 26)
+#define AHBDMA_MST_ID MST_ID(5)
+#define USB_MST_ID MST_ID(6)
+#define USB2_MST_ID MST_ID(18)
+#define USB3_MST_ID MST_ID(17)
+#define ADDR_BNDRY(x) (((x) & 0xf) << 21)
+#define INACTIVITY_TIMEOUT(x) (((x) & 0xffff) << 0)
+
+#define AHB_ARBITRATION_AHB_MEM_WRQUE_MST_ID 0xf8
+
+static u32 tegra_ahb_gizmo[] = {
+ AHB_ARBITRATION_DISABLE,
+ AHB_ARBITRATION_PRIORITY_CTRL,
+ AHB_GIZMO_AHB_MEM,
+ AHB_GIZMO_APB_DMA,
+ AHB_GIZMO_IDE,
+ AHB_GIZMO_USB,
+ AHB_GIZMO_AHB_XBAR_BRIDGE,
+ AHB_GIZMO_CPU_AHB_BRIDGE,
+ AHB_GIZMO_COP_AHB_BRIDGE,
+ AHB_GIZMO_XBAR_APB_CTLR,
+ AHB_GIZMO_VCP_AHB_BRIDGE,
+ AHB_GIZMO_NAND,
+ AHB_GIZMO_SDMMC4,
+ AHB_GIZMO_XIO,
+ AHB_GIZMO_BSEV,
+ AHB_GIZMO_BSEA,
+ AHB_GIZMO_NOR,
+ AHB_GIZMO_USB2,
+ AHB_GIZMO_USB3,
+ AHB_GIZMO_SDMMC1,
+ AHB_GIZMO_SDMMC2,
+ AHB_GIZMO_SDMMC3,
+ AHB_MEM_PREFETCH_CFG_X,
+ AHB_ARBITRATION_XBAR_CTRL,
+ AHB_MEM_PREFETCH_CFG3,
+ AHB_MEM_PREFETCH_CFG4,
+ AHB_MEM_PREFETCH_CFG1,
+ AHB_MEM_PREFETCH_CFG2,
+ AHB_ARBITRATION_AHB_MEM_WRQUE_MST_ID,
+};
+
+struct tegra_ahb {
+ void __iomem *regs;
+ struct device *dev;
+ u32 ctx[0];
+};
+
+static inline u32 gizmo_readl(struct tegra_ahb *ahb, u32 offset)
+{
+ return readl(ahb->regs + offset);
+}
+
+static inline void gizmo_writel(struct tegra_ahb *ahb, u32 value, u32 offset)
+{
+ writel(value, ahb->regs + offset);
+}
+
+static int tegra_ahb_suspend(struct device *dev)
+{
+ int i;
+ struct tegra_ahb *ahb = dev_get_drvdata(dev);
+
+ for (i = 0; i < ARRAY_SIZE(tegra_ahb_gizmo); i++)
+ ahb->ctx[i] = gizmo_readl(ahb, tegra_ahb_gizmo[i]);
+ return 0;
+}
+
+static int tegra_ahb_resume(struct device *dev)
+{
+ int i;
+ struct tegra_ahb *ahb = dev_get_drvdata(dev);
+
+ for (i = 0; i < ARRAY_SIZE(tegra_ahb_gizmo); i++)
+ gizmo_writel(ahb, ahb->ctx[i], tegra_ahb_gizmo[i]);
+ return 0;
+}
+
+static UNIVERSAL_DEV_PM_OPS(tegra_ahb_pm,
+ tegra_ahb_suspend,
+ tegra_ahb_resume, NULL);
+
+static void tegra_ahb_gizmo_init(struct tegra_ahb *ahb)
+{
+ u32 val;
+
+ val = gizmo_readl(ahb, AHB_GIZMO_AHB_MEM);
+ val |= ENB_FAST_REARBITRATE | IMMEDIATE | DONT_SPLIT_AHB_WR;
+ gizmo_writel(ahb, val, AHB_GIZMO_AHB_MEM);
+
+ val = gizmo_readl(ahb, AHB_GIZMO_USB);
+ val |= IMMEDIATE;
+ gizmo_writel(ahb, val, AHB_GIZMO_USB);
+
+ val = gizmo_readl(ahb, AHB_GIZMO_USB2);
+ val |= IMMEDIATE;
+ gizmo_writel(ahb, val, AHB_GIZMO_USB2);
+
+ val = gizmo_readl(ahb, AHB_GIZMO_USB3);
+ val |= IMMEDIATE;
+ gizmo_writel(ahb, val, AHB_GIZMO_USB3);
+
+ val = gizmo_readl(ahb, AHB_ARBITRATION_PRIORITY_CTRL);
+ val |= PRIORITY_SELECT_USB |
+ PRIORITY_SELECT_USB2 |
+ PRIORITY_SELECT_USB3 |
+ AHB_PRIORITY_WEIGHT(7);
+ gizmo_writel(ahb, val, AHB_ARBITRATION_PRIORITY_CTRL);
+
+ val = gizmo_readl(ahb, AHB_MEM_PREFETCH_CFG1);
+ val &= ~MST_ID(~0);
+ val |= PREFETCH_ENB |
+ AHBDMA_MST_ID |
+ ADDR_BNDRY(0xc) |
+ INACTIVITY_TIMEOUT(0x1000);
+ gizmo_writel(ahb, val, AHB_MEM_PREFETCH_CFG1);
+
+ val = gizmo_readl(ahb, AHB_MEM_PREFETCH_CFG2);
+ val &= ~MST_ID(~0);
+ val |= PREFETCH_ENB |
+ USB_MST_ID |
+ ADDR_BNDRY(0xc) |
+ INACTIVITY_TIMEOUT(0x1000);
+ gizmo_writel(ahb, val, AHB_MEM_PREFETCH_CFG2);
+
+ val = gizmo_readl(ahb, AHB_MEM_PREFETCH_CFG3);
+ val &= ~MST_ID(~0);
+ val |= PREFETCH_ENB |
+ USB3_MST_ID |
+ ADDR_BNDRY(0xc) |
+ INACTIVITY_TIMEOUT(0x1000);
+ gizmo_writel(ahb, val, AHB_MEM_PREFETCH_CFG3);
+
+ val = gizmo_readl(ahb, AHB_MEM_PREFETCH_CFG4);
+ val &= ~MST_ID(~0);
+ val |= PREFETCH_ENB |
+ USB2_MST_ID |
+ ADDR_BNDRY(0xc) |
+ INACTIVITY_TIMEOUT(0x1000);
+ gizmo_writel(ahb, val, AHB_MEM_PREFETCH_CFG4);
+}
+
+static int __devinit tegra_ahb_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct tegra_ahb *ahb;
+ size_t bytes;
+
+ bytes = sizeof(*ahb) + sizeof(u32) * ARRAY_SIZE(tegra_ahb_gizmo);
+ ahb = devm_kzalloc(&pdev->dev, bytes, GFP_KERNEL);
+ if (!ahb)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+ ahb->regs = devm_request_and_ioremap(&pdev->dev, res);
+ if (!ahb->regs)
+ return -EBUSY;
+
+ ahb->dev = &pdev->dev;
+ platform_set_drvdata(pdev, ahb);
+ tegra_ahb_gizmo_init(ahb);
+ return 0;
+}
+
+static int __devexit tegra_ahb_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id tegra_ahb_of_match[] __devinitconst = {
+ { .compatible = "nvidia,tegra30-ahb", },
+ { .compatible = "nvidia,tegra20-ahb", },
+ {},
+};
+
+static struct platform_driver tegra_ahb_driver = {
+ .probe = tegra_ahb_probe,
+ .remove = __devexit_p(tegra_ahb_remove),
+ .driver = {
+ .name = DRV_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = tegra_ahb_of_match,
+ .pm = &tegra_ahb_pm,
+ },
+};
+
+static int __init tegra_ahb_module_init(void)
+{
+ return platform_driver_register(&tegra_ahb_driver);
+}
+postcore_initcall(tegra_ahb_module_init);
+
+static void __exit tegra_ahb_module_exit(void)
+{
+ platform_driver_unregister(&tegra_ahb_driver);
+}
+module_exit(tegra_ahb_module_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU <[email protected]>");
+MODULE_DESCRIPTION("Tegra AHB driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
--
1.7.5.4


2012-05-03 16:06:39

by Hiroshi Doyu

[permalink] [raw]
Subject: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
ready, instead of directly aceessing AHB registers.

Signed-off-by: Hiroshi DOYU <[email protected]>
---
drivers/iommu/tegra-smmu.c | 60 +++++++++++++++++--------------------------
1 files changed, 24 insertions(+), 36 deletions(-)

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index ecd6790..c70e4e7 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -30,12 +30,14 @@
#include <linux/sched.h>
#include <linux/iommu.h>
#include <linux/io.h>
+#include <linux/of.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,11 +113,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 */
@@ -235,7 +232,7 @@ struct smmu_as {
* Per SMMU device - IOMMU device
*/
struct smmu_device {
- void __iomem *regs, *regs_ahbarb;
+ void __iomem *regs;
unsigned long iovmm_base; /* remappable base address */
unsigned long page_count; /* total remappable size */
spinlock_t lock;
@@ -252,12 +249,14 @@ 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)
{
@@ -268,15 +267,6 @@ static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
writel(val, smmu->regs + offs);
}

-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)
-{
- writel(val, smmu->regs_ahbarb + offs);
-}
-
#define VA_PAGE_TO_PA(va, page) \
(page_to_phys(page) + ((unsigned long)(va) & ~PAGE_MASK))

@@ -370,9 +360,9 @@ 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;
+ int i, err;
u32 val;

for (i = 0; i < smmu->num_as; i++) {
@@ -398,10 +388,8 @@ 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);
+ err = tegra_ahb_enable_smmu(smmu->ahb);
+ return err;
}

static void flush_ptc_and_tlb(struct smmu_device *smmu,
@@ -873,17 +861,18 @@ 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 resource *regs, *window;
struct device *dev = &pdev->dev;
int i, err = 0;

@@ -893,9 +882,8 @@ 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);
- regs2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- window = platform_get_resource(pdev, IORESOURCE_MEM, 2);
- if (!regs || !regs2 || !window) {
+ window = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ if (!regs || !window) {
dev_err(dev, "No SMMU resources\n");
return -ENODEV;
}
@@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)
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) {
+ if (!smmu->regs) {
dev_err(dev, "failed to remap SMMU registers\n");
err = -ENXIO;
goto fail;
}

+ smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);
+ if (!smmu->ahb)
+ return -ENODEV;
+
smmu->translation_enable_0 = ~0;
smmu->translation_enable_1 = ~0;
smmu->translation_enable_2 = ~0;
@@ -945,7 +935,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);
@@ -960,8 +952,6 @@ fail:
__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 +983,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;
--
1.7.5.4

2012-05-03 16:06:23

by Hiroshi Doyu

[permalink] [raw]
Subject: [PATCHv3 2/4] ARM: tegra: Add SMMU enabler in AHB

Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
ready.

Signed-off-by: Hiroshi DOYU <[email protected]>
Cc: Felipe Balbi <[email protected]>
---
arch/arm/mach-tegra/include/mach/tegra-ahb.h | 19 +++++++++++++++
drivers/amba/tegra-ahb.c | 32 ++++++++++++++++++++++++++
2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-tegra/include/mach/tegra-ahb.h b/arch/arm/mach-tegra/include/mach/tegra-ahb.h
new file mode 100644
index 0000000..e0f8c84
--- /dev/null
+++ b/arch/arm/mach-tegra/include/mach/tegra-ahb.h
@@ -0,0 +1,19 @@
+/*
+ * 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.
+ */
+
+#ifndef __MACH_TEGRA_AHB_H__
+#define __MACH_TEGRA_AHB_H__
+
+extern int tegra_ahb_enable_smmu(struct device_node *ahb);
+
+#endif /* __MACH_TEGRA_AHB_H__ */
diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
index 03933f3..ec4e22f 100644
--- a/drivers/amba/tegra-ahb.c
+++ b/drivers/amba/tegra-ahb.c
@@ -76,6 +76,10 @@

#define AHB_ARBITRATION_AHB_MEM_WRQUE_MST_ID 0xf8

+#define AHB_ARBITRATION_XBAR_CTRL_SMMU_INIT_DONE BIT(17)
+
+static struct platform_driver tegra_ahb_driver;
+
static u32 tegra_ahb_gizmo[] = {
AHB_ARBITRATION_DISABLE,
AHB_ARBITRATION_PRIORITY_CTRL,
@@ -124,6 +128,34 @@ static inline void gizmo_writel(struct tegra_ahb *ahb, u32 value, u32 offset)
writel(value, ahb->regs + offset);
}

+#ifdef CONFIG_ARCH_TEGRA_3x_SOC
+static int tegra_ahb_match_by_smmu(struct device *dev, void *data)
+{
+ struct tegra_ahb *ahb = dev_get_drvdata(dev);
+ struct device_node *dn = data;
+
+ return (ahb->dev->of_node == dn) ? 1 : 0;
+}
+
+int tegra_ahb_enable_smmu(struct device_node *dn)
+{
+ struct device *dev;
+ u32 val;
+ struct tegra_ahb *ahb;
+
+ dev = driver_find_device(&tegra_ahb_driver.driver, NULL, dn,
+ tegra_ahb_match_by_smmu);
+ if (!dev)
+ return -EPROBE_DEFER;
+ ahb = dev_get_drvdata(dev);
+ val = gizmo_readl(ahb, AHB_ARBITRATION_XBAR_CTRL);
+ val |= AHB_ARBITRATION_XBAR_CTRL_SMMU_INIT_DONE;
+ gizmo_writel(ahb, val, AHB_ARBITRATION_XBAR_CTRL);
+ return 0;
+}
+EXPORT_SYMBOL(tegra_ahb_enable_smmu);
+#endif
+
static int tegra_ahb_suspend(struct device *dev)
{
int i;
--
1.7.5.4

2012-05-03 16:06:50

by Hiroshi Doyu

[permalink] [raw]
Subject: [PATCHv3 4/4] ARM: dt: tegra: Add device tree support for AHB

Add AHB entry for tegra20/30.

Signed-off-by: Hiroshi DOYU <[email protected]>
---
arch/arm/boot/dts/tegra20.dtsi | 5 +++++
arch/arm/boot/dts/tegra30.dtsi | 5 +++++
2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 52cab08..ea6de57 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -212,5 +212,10 @@
reg = < 0x7000f000 0x00000100 /* controller registers */
0x58000000 0x02000000 >; /* GART aperture */
};
+
+ ahb: ahb@6000c004 {
+ compatible = "nvidia,tegra20-ahb";
+ reg = <0x6000c004 0x10c>; /* AHB Arbitration + Gizmo Controller */
+ };
};

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 15200a9..9d650fb 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -224,4 +224,9 @@
nvidia,ahub-cif-ids = <8 8>;
};
};
+
+ ahb: ahb@6000c004 {
+ compatible = "nvidia,tegra30-ahb";
+ reg = <0x6000c004 0x14c>; /* AHB Arbitration + Gizmo Controller */
+ };
};
--
1.7.5.4

2012-05-03 17:41:42

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
> High-performance Bus (AHB) architecture.
>
> The AHB Arbiter controls AHB bus master arbitration. This effectively
> forms a second level of arbitration for access to the memory
> controller through the AHB Slave Memory device. The AHB pre-fetch
> logic can be configured to enhance performance for devices doing
> sequential access. Each AHB master is assigned to either the high or
> low priority bin. Both Tegra20/30 have this AHB bus.
>
> Some of configuration param could be passed from DT too.

> diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
> +static u32 tegra_ahb_gizmo[] = {

const?

> +static int __init tegra_ahb_module_init(void)
> +{
> + return platform_driver_register(&tegra_ahb_driver);
> +}
> +postcore_initcall(tegra_ahb_module_init);

Can this be a module_init() instead of postcore_initcall()?

> +
> +static void __exit tegra_ahb_module_exit(void)
> +{
> + platform_driver_unregister(&tegra_ahb_driver);
> +}
> +module_exit(tegra_ahb_module_exit);

If so, all of the previous two quoted chunks can be replaced with just:

module_platform_driver(tegra_ahb_module_init);

2012-05-03 17:43:44

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 2/4] ARM: tegra: Add SMMU enabler in AHB

On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> ready.
>
> Signed-off-by: Hiroshi DOYU <[email protected]>
> Cc: Felipe Balbi <[email protected]>

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

2012-05-03 17:48:29

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> ready, instead of directly aceessing AHB registers.

You need to make the Kconfig option for the SMMU either depend on or
select the TEGRA_AHB option. If you don't, then if someone disables the
AHB driver, the SMMU driver may still build, yet fail to link since the
AHB API it calls doesn't exist.

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

> @@ -398,10 +388,8 @@ 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);
> + err = tegra_ahb_enable_smmu(smmu->ahb);
> + return err;

You can just "return tegra_ahb_..." here.

> @@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)

> + smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);

Hmm, "ahb" should probably be "nvidia,ahb".

I see that neither this patch nor the next patch include binding
documentation that describe this property. Can you please add documentation.

2012-05-03 17:50:57

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 4/4] ARM: dt: tegra: Add device tree support for AHB

On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> Add AHB entry for tegra20/30.

This looks OK. I can take it through the Tegra tree once the review
issues are fixed.

2012-05-03 17:54:46

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

From: Stephen Warren <[email protected]>
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB
Date: Thu, 3 May 2012 19:48:25 +0200
Message-ID: <[email protected]>

> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> > Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> > ready, instead of directly aceessing AHB registers.
>
> You need to make the Kconfig option for the SMMU either depend on or
> select the TEGRA_AHB option. If you don't, then if someone disables the
> AHB driver, the SMMU driver may still build, yet fail to link since the
> AHB API it calls doesn't exist.
>
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>
> > @@ -398,10 +388,8 @@ 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);
> > + err = tegra_ahb_enable_smmu(smmu->ahb);
> > + return err;
>
> You can just "return tegra_ahb_..." here.
>
> > @@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>
> > + smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);
>
> Hmm, "ahb" should probably be "nvidia,ahb".
>
> I see that neither this patch nor the next patch include binding
> documentation that describe this property. Can you please add documentation.

There will be the tegra Memory Controller(MC) patches following, which
will change the way DT is passed, where SMMU/dt is passed from
MC.

Can we wait for the following MC patches, not haveing doc for this
patch?

2012-05-03 17:58:35

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

On 05/03/2012 11:48 AM, Stephen Warren wrote:
> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
>> Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
>> ready, instead of directly aceessing AHB registers.

Oh, that should be "accessing".

>> @@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>
>> + smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);
>
> Hmm, "ahb" should probably be "nvidia,ahb".
>
> I see that neither this patch nor the next patch include binding
> documentation that describe this property. Can you please add documentation.

Oh, the next patch is just adding the entry to the .dtsi file for the
AHB, so no surprise it doesn't add a binding document for the SMMU!

I see that with this patch, the driver still expects the DMA window to
be represented as a reg property (IORESOURCE_MEM), so if we add a
binding document to this patch it won't be very consistent either:-( And
then, there's the issue of whether the SMMU should be it's own device or
a child of some MC device, since there's non-SMMU functionality in these
registers too. This makes all the SMMU rework need a little more thought.

So, I propose dropping this patch from this series, since this series is
all about adding the AHB driver. We should move this patch to a series
relating to the SMMU driver.

2012-05-03 17:59:38

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
> High-performance Bus (AHB) architecture.
>
> The AHB Arbiter controls AHB bus master arbitration. This effectively
> forms a second level of arbitration for access to the memory
> controller through the AHB Slave Memory device. The AHB pre-fetch
> logic can be configured to enhance performance for devices doing
> sequential access. Each AHB master is assigned to either the high or
> low priority bin. Both Tegra20/30 have this AHB bus.
>
> Some of configuration param could be passed from DT too.

This patch should add
Documentation/devicetree/bindings/arm/tegra/tegra20-ahb.txt to describe
the DT binding.

2012-05-04 06:17:46

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

From: Stephen Warren <[email protected]>
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver
Date: Thu, 3 May 2012 19:41:35 +0200
Message-ID: <[email protected]>

> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> > Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
> > High-performance Bus (AHB) architecture.
> >
> > The AHB Arbiter controls AHB bus master arbitration. This effectively
> > forms a second level of arbitration for access to the memory
> > controller through the AHB Slave Memory device. The AHB pre-fetch
> > logic can be configured to enhance performance for devices doing
> > sequential access. Each AHB master is assigned to either the high or
> > low priority bin. Both Tegra20/30 have this AHB bus.
> >
> > Some of configuration param could be passed from DT too.
>
> > diff --git a/drivers/amba/tegra-ahb.c b/drivers/amba/tegra-ahb.c
> > +static u32 tegra_ahb_gizmo[] = {
>
> const?

Yes.

> > +static int __init tegra_ahb_module_init(void)
> > +{
> > + return platform_driver_register(&tegra_ahb_driver);
> > +}
> > +postcore_initcall(tegra_ahb_module_init);
>
> Can this be a module_init() instead of postcore_initcall()?

Since this driver configures prefetch size from AHB client devices,
it's better to make this driver available before other AHB client
drivers get ready. So "postcore_initcall()" seems to make sense if
there's no other better initcall.

> > +
> > +static void __exit tegra_ahb_module_exit(void)
> > +{
> > + platform_driver_unregister(&tegra_ahb_driver);
> > +}
> > +module_exit(tegra_ahb_module_exit);
>
> If so, all of the previous two quoted chunks can be replaced with just:
>
> module_platform_driver(tegra_ahb_module_init);

2012-05-04 06:34:13

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

From: Stephen Warren <[email protected]>
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB
Date: Thu, 3 May 2012 19:48:25 +0200
Message-ID: <[email protected]>

> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> > Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> > ready, instead of directly aceessing AHB registers.
>
> You need to make the Kconfig option for the SMMU either depend on or
> select the TEGRA_AHB option. If you don't, then if someone disables the
> AHB driver, the SMMU driver may still build, yet fail to link since the
> AHB API it calls doesn't exist.
>
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>
> > @@ -398,10 +388,8 @@ 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);
> > + err = tegra_ahb_enable_smmu(smmu->ahb);
> > + return err;
>
> You can just "return tegra_ahb_..." here.
>
> > @@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>
> > + smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);
>
> Hmm, "ahb" should probably be "nvidia,ahb".

Does this mean the following in dts?

ahb: ahb@6000c004 {
compatible = "nvidia,tegra30-ahb";
reg = <0x6000c004 0x14c>; /* AHB Arbitration + Gizmo Controller */
};

smmu: smmu@7000f000 {
nvidia,ahb = &ahb;
}; ^^^^^^^^^^

2012-05-04 06:36:49

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

From: Stephen Warren <[email protected]>
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB
Date: Thu, 3 May 2012 19:58:30 +0200
Message-ID: <[email protected]>

> On 05/03/2012 11:48 AM, Stephen Warren wrote:
> > On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> >> Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> >> ready, instead of directly aceessing AHB registers.
>
> Oh, that should be "accessing".
>
> >> @@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)
> >
> >> + smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);
> >
> > Hmm, "ahb" should probably be "nvidia,ahb".
> >
> > I see that neither this patch nor the next patch include binding
> > documentation that describe this property. Can you please add documentation.
>
> Oh, the next patch is just adding the entry to the .dtsi file for the
> AHB, so no surprise it doesn't add a binding document for the SMMU!
>
> I see that with this patch, the driver still expects the DMA window to
> be represented as a reg property (IORESOURCE_MEM), so if we add a
> binding document to this patch it won't be very consistent either:-( And
> then, there's the issue of whether the SMMU should be it's own device or
> a child of some MC device, since there's non-SMMU functionality in these
> registers too. This makes all the SMMU rework need a little more thought.
>
> So, I propose dropping this patch from this series, since this series is
> all about adding the AHB driver. We should move this patch to a series
> relating to the SMMU driver.

Ok, I'll post the first 3 patches.

2012-05-04 06:40:45

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

From: Stephen Warren <[email protected]>
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver
Date: Thu, 3 May 2012 19:59:33 +0200
Message-ID: <[email protected]>

> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> > Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
> > High-performance Bus (AHB) architecture.
> >
> > The AHB Arbiter controls AHB bus master arbitration. This effectively
> > forms a second level of arbitration for access to the memory
> > controller through the AHB Slave Memory device. The AHB pre-fetch
> > logic can be configured to enhance performance for devices doing
> > sequential access. Each AHB master is assigned to either the high or
> > low priority bin. Both Tegra20/30 have this AHB bus.
> >
> > Some of configuration param could be passed from DT too.
>
> This patch should add
> Documentation/devicetree/bindings/arm/tegra/tegra20-ahb.txt to describe
> the DT binding.

>From 'dts' POV, there's no difference between tegra20 and
tegra30. They just have a register range. Is
"../bindings/arm/tegra/tegra-ahb.txt" ok, instead of having both
tegra{20,30}-ahb.txt?

2012-05-04 12:23:12

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

On Friday 04 May 2012, Hiroshi Doyu wrote:
> > This patch should add
> > Documentation/devicetree/bindings/arm/tegra/tegra20-ahb.txt to describe
> > the DT binding.
>
> From 'dts' POV, there's no difference between tegra20 and
> tegra30. They just have a register range. Is
> "../bindings/arm/tegra/tegra-ahb.txt" ok, instead of having both
> tegra{20,30}-ahb.txt?

I think either one is ok. Just calling it tegra20-ahb.txt would be
fine too because tegra30 is compatible to that.

Arnd

2012-05-04 16:49:03

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

On 05/04/2012 12:17 AM, Hiroshi Doyu wrote:
> Stephen Warren wrote at Thu, 3 May 2012 19:41:35 +0200:
>> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
>>> Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
>>> High-performance Bus (AHB) architecture.
...
>>> +static int __init tegra_ahb_module_init(void)
>>> +{
>>> + return platform_driver_register(&tegra_ahb_driver);
>>> +}
>>> +postcore_initcall(tegra_ahb_module_init);
>>
>> Can this be a module_init() instead of postcore_initcall()?
>
> Since this driver configures prefetch size from AHB client devices,
> it's better to make this driver available before other AHB client
> drivers get ready. So "postcore_initcall()" seems to make sense if
> there's no other better initcall.

I believe this only affects when the driver is registered and has no
influence over when the device itself is probed.

When booting with board files rather than DT, it was possible to
register drivers and platform devices early using various initcalls to
control the order. However, with DT, I believe all the devices are
instantiated from DT at the same time (well, one by one in whatever
order as the DT is parsed), so the time when the driver is registered
isn't relevant.

So, if the other AHB devices really need this AHB driver to initialize
first, you'd better move all the AHB devices inside the AHB node in DT,
so that the AHB driver can influence when the children get probed.
Still, I'd suggest leaving that to a later patch, since everything
clearly works fine right now without this driver even existing.

>>> +
>>> +static void __exit tegra_ahb_module_exit(void)
>>> +{
>>> + platform_driver_unregister(&tegra_ahb_driver);
>>> +}
>>> +module_exit(tegra_ahb_module_exit);
>>
>> If so, all of the previous two quoted chunks can be replaced with just:
>>
>> module_platform_driver(tegra_ahb_module_init);

2012-05-04 16:50:25

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

On 05/04/2012 12:40 AM, Hiroshi Doyu wrote:
> From: Stephen Warren <[email protected]>
> Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver
> Date: Thu, 3 May 2012 19:59:33 +0200
> Message-ID: <[email protected]>
>
>> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
>>> Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
>>> High-performance Bus (AHB) architecture.
>>>
>>> The AHB Arbiter controls AHB bus master arbitration. This effectively
>>> forms a second level of arbitration for access to the memory
>>> controller through the AHB Slave Memory device. The AHB pre-fetch
>>> logic can be configured to enhance performance for devices doing
>>> sequential access. Each AHB master is assigned to either the high or
>>> low priority bin. Both Tegra20/30 have this AHB bus.
>>>
>>> Some of configuration param could be passed from DT too.
>>
>> This patch should add
>> Documentation/devicetree/bindings/arm/tegra/tegra20-ahb.txt to describe
>> the DT binding.
>
> From 'dts' POV, there's no difference between tegra20 and
> tegra30. They just have a register range. Is
> "../bindings/arm/tegra/tegra-ahb.txt" ok, instead of having both
> tegra{20,30}-ahb.txt?

Personally, I prefer to name files using the exact compatible value of
the first SoC version that introduced the HW. This allows e.g.
"tegra40-ahb.txt" to be introduced without making "tegra-ahb.txt" no
longer fully general.

2012-05-04 16:51:46

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB

On 05/04/2012 12:33 AM, Hiroshi Doyu wrote:
> Stephen Warren wrote at Thu, 3 May 2012 19:48:25 +0200:
>> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
>>> Use "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
>>> ready, instead of directly aceessing AHB registers.
...
>>> @@ -911,14 +899,16 @@ static int tegra_smmu_probe(struct platform_device *pdev)
>>
>>> + smmu->ahb = of_parse_phandle(pdev->dev.of_node, "ahb", 0);
>>
>> Hmm, "ahb" should probably be "nvidia,ahb".
>
> Does this mean the following in dts?
>
> ahb: ahb@6000c004 {
> compatible = "nvidia,tegra30-ahb";
> reg = <0x6000c004 0x14c>; /* AHB Arbitration + Gizmo Controller */
> };
>
> smmu: smmu@7000f000 {
> nvidia,ahb = &ahb;
> }; ^^^^^^^^^^
>

Yes.

2012-05-07 06:07:17

by Hiroshi Doyu

[permalink] [raw]
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add Tegra AHB driver

On Fri, 4 May 2012 18:48:56 +0200
Stephen Warren <[email protected]> wrote:

> On 05/04/2012 12:17 AM, Hiroshi Doyu wrote:
> > Stephen Warren wrote at Thu, 3 May 2012 19:41:35 +0200:
> >> On 05/03/2012 10:05 AM, Hiroshi DOYU wrote:
> >>> Tegra AHB Bus conforms to the AMBA Specification (Rev 2.0) Advanced
> >>> High-performance Bus (AHB) architecture.
> ...
> >>> +static int __init tegra_ahb_module_init(void)
> >>> +{
> >>> + return platform_driver_register(&tegra_ahb_driver);
> >>> +}
> >>> +postcore_initcall(tegra_ahb_module_init);
> >>
> >> Can this be a module_init() instead of postcore_initcall()?
> >
> > Since this driver configures prefetch size from AHB client devices,
> > it's better to make this driver available before other AHB client
> > drivers get ready. So "postcore_initcall()" seems to make sense if
> > there's no other better initcall.
>
> I believe this only affects when the driver is registered and has no
> influence over when the device itself is probed.
>
> When booting with board files rather than DT, it was possible to
> register drivers and platform devices early using various initcalls to
> control the order. However, with DT, I believe all the devices are
> instantiated from DT at the same time (well, one by one in whatever
> order as the DT is parsed), so the time when the driver is registered
> isn't relevant.
>
> So, if the other AHB devices really need this AHB driver to initialize
> first, you'd better move all the AHB devices inside the AHB node in DT,
> so that the AHB driver can influence when the children get probed.
> Still, I'd suggest leaving that to a later patch, since everything
> clearly works fine right now without this driver even existing.

Ok, I'll use module_platform_driver(tegra_ahb_driver). These order would be revisited.