[PATCHv3 1/4] ARM: tegra: Add AHB driver

[PATCHv3 1/4] ARM: tegra: Add AHB driver

[Hiroshi DOYU]

The 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 <hdoyu@nvidia.com>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
v4:
Fixed the comments from Felipe/Russell.
This is now located under drivers/platform/arm.
v3:
Use platform_device to get info from dt dynamically.(Felipe/Arnd)

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
 drivers/platform/Kconfig         |    5 +
 drivers/platform/Makefile        |    1 +
 drivers/platform/arm/Kconfig     |    8 +
 drivers/platform/arm/Makefile    |    5 +
 drivers/platform/arm/tegra-ahb.c |  278 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 297 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 8390dca..8b96a77 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -1,3 +1,8 @@
 if X86
 source "drivers/platform/x86/Kconfig"
 endif
+
+if ARM
+source "drivers/platform/arm/Kconfig"
+endif
+
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index 782953a..7a2b16f 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-$(CONFIG_X86)		+= x86/
+obj-$(CONFIG_ARM)		+= arm/
diff --git a/drivers/platform/arm/Kconfig b/drivers/platform/arm/Kconfig
new file mode 100644
index 0000000..0af31db
--- /dev/null
+++ b/drivers/platform/arm/Kconfig
@@ -0,0 +1,8 @@
+config TEGRA_AHB
+	bool "Enable AHB driver for NVIDIA Tegra SoCs"
+	depends on ARCH_TEGRA
+	default y
+	help
+	  Adds AHB configuration functionality for NVIDIA Tegra SoCs,
+	  which controls AHB bus master arbitration and some
+	  perfomance parameters(priority, prefech size).
diff --git a/drivers/platform/arm/Makefile b/drivers/platform/arm/Makefile
new file mode 100644
index 0000000..3535f56
--- /dev/null
+++ b/drivers/platform/arm/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for linux/drivers/platform/arm
+# ARM Platform-Specific Drivers
+#
+obj-$(CONFIG_TEGRA_AHB)		+= tegra-ahb.o
diff --git a/drivers/platform/arm/tegra-ahb.c b/drivers/platform/arm/tegra-ahb.c
new file mode 100644
index 0000000..dcad2ec
--- /dev/null
+++ b/drivers/platform/arm/tegra-ahb.c
@@ -0,0 +1,278 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ * Copyright (C) 2011 Google, Inc.
+ *
+ * Author:
+ *	Jay Cheng <jacheng@nvidia.com>
+ *	James Wylder <james.wylder@motorola.com>
+ *	Benoit Goby <benoit@android.com>
+ *	Colin Cross <ccross@android.com>
+ *	Hiroshi DOYU <hdoyu@nvidia.com>
+ *
+ * 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>
+
+#include <mach/iomap.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;
+	void		*ctx;
+	struct device	*dev;
+};
+
+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);
+	u32 *p = ahb->ctx;
+
+	for (i = 0; i < ARRAY_SIZE(tegra_ahb_gizmo); i++)
+		p[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);
+	u32 *p = ahb->ctx;
+
+	for (i = 0; i < ARRAY_SIZE(tegra_ahb_gizmo); i++)
+		gizmo_writel(ahb, p[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;
+	ahb->ctx = ahb + 1;
+
+	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)
+{
+	platform_set_drvdata(pdev, NULL);
+	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_init(void)
+{
+	return platform_driver_register(&tegra_ahb_driver);
+}
+postcore_initcall(tegra_ahb_init);
+
+static void __exit tegra_ahb_exit(void)
+{
+	platform_driver_unregister(&tegra_ahb_driver);
+}
+module_exit(tegra_ahb_exit);
+
+MODULE_AUTHOR("Hiroshi DOYU <hdoyu@nvidia.com>");
+MODULE_DESCRIPTION("Tegra AHB driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
1.7.5.4

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

[Hiroshi DOYU]

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

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
Cc: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-tegra/include/mach/tegra-ahb.h |   19 +++++++++++++++++++
 drivers/platform/arm/tegra-ahb.c             |   24 ++++++++++++++++++++++++
 2 files changed, 43 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..296688c
--- /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(void);
+
+#endif	/* __MACH_TEGRA_AHB_H__ */
diff --git a/drivers/platform/arm/tegra-ahb.c b/drivers/platform/arm/tegra-ahb.c
index dcad2ec..7ad9dfd 100644
--- a/drivers/platform/arm/tegra-ahb.c
+++ b/drivers/platform/arm/tegra-ahb.c
@@ -78,6 +78,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,
@@ -126,6 +130,26 @@ 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_enable_smmu(struct device *dev, void *data)
+{
+	u32 val;
+	struct tegra_ahb *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;
+}
+
+int tegra_ahb_enable_smmu(void)
+{
+	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
+				      __tegra_ahb_enable_smmu);
+}
+EXPORT_SYMBOL(tegra_ahb_enable_smmu);
+#endif
+
 static int tegra_ahb_suspend(struct device *dev)
 {
 	int i;
-- 
1.7.5.4

[PATCHv3 3/4] iommu/tegra: smmu: Refrain from accessing to AHB registers

[Hiroshi DOYU]

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

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

diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index ecd6790..de9fafe 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -36,6 +36,7 @@
 
 #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 +112,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 +231,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;
@@ -257,7 +253,7 @@ struct smmu_device {
 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 +264,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))
 
@@ -397,11 +384,7 @@ static void smmu_setup_regs(struct smmu_device *smmu)
 	smmu_write(smmu, SMMU_PTC_CONFIG_RESET_VAL, SMMU_PTC_CONFIG);
 
 	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);
+	tegra_ahb_enable_smmu();
 }
 
 static void flush_ptc_and_tlb(struct smmu_device *smmu,
@@ -883,7 +866,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, *regs2, *window;
+	struct resource *regs, *window;
 	struct device *dev = &pdev->dev;
 	int i, err = 0;
 
@@ -893,9 +876,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,9 +893,7 @@ 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;
@@ -960,8 +940,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 +971,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

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

[Hiroshi DOYU]

Add AHB entry for tegra20/30.

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
---
 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..54e9640 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,tegra30-ahb", "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..24120cf 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", "nvidia,tegra20-ahb";
+		reg = <0x6000c004 0x14c>; /* AHB Arbitration + Gizmo Controller */
+	};
 };
-- 
1.7.5.4

Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 11575 bytes --]

On Wed, Apr 25, 2012 at 02:07:36PM +0300, Hiroshi DOYU wrote:
> The 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 <hdoyu@nvidia.com>
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> ---
> v4:
> Fixed the comments from Felipe/Russell.
> This is now located under drivers/platform/arm.
> v3:
> Use platform_device to get info from dt dynamically.(Felipe/Arnd)
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>

Just one thing, you seem to have missed Russell's comment about this
driver being generic. You still add a TEGRA_AHB driver, also, you don't
need <mach/iomap.h>. Apart from those, you can add:

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  drivers/platform/Kconfig         |    5 +
>  drivers/platform/Makefile        |    1 +
>  drivers/platform/arm/Kconfig     |    8 +
>  drivers/platform/arm/Makefile    |    5 +
>  drivers/platform/arm/tegra-ahb.c |  278 ++++++++++++++++++++++++++++++++++++++
>  5 files changed, 297 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> index 8390dca..8b96a77 100644
> --- a/drivers/platform/Kconfig
> +++ b/drivers/platform/Kconfig
> @@ -1,3 +1,8 @@
>  if X86
>  source "drivers/platform/x86/Kconfig"
>  endif
> +
> +if ARM
> +source "drivers/platform/arm/Kconfig"
> +endif
> +
> diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> index 782953a..7a2b16f 100644
> --- a/drivers/platform/Makefile
> +++ b/drivers/platform/Makefile
> @@ -3,3 +3,4 @@
>  #
>  
>  obj-$(CONFIG_X86)		+= x86/
> +obj-$(CONFIG_ARM)		+= arm/
> diff --git a/drivers/platform/arm/Kconfig b/drivers/platform/arm/Kconfig
> new file mode 100644
> index 0000000..0af31db
> --- /dev/null
> +++ b/drivers/platform/arm/Kconfig
> @@ -0,0 +1,8 @@
> +config TEGRA_AHB
> +	bool "Enable AHB driver for NVIDIA Tegra SoCs"
> +	depends on ARCH_TEGRA
> +	default y
> +	help
> +	  Adds AHB configuration functionality for NVIDIA Tegra SoCs,
> +	  which controls AHB bus master arbitration and some
> +	  perfomance parameters(priority, prefech size).
> diff --git a/drivers/platform/arm/Makefile b/drivers/platform/arm/Makefile
> new file mode 100644
> index 0000000..3535f56
> --- /dev/null
> +++ b/drivers/platform/arm/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for linux/drivers/platform/arm
> +# ARM Platform-Specific Drivers
> +#
> +obj-$(CONFIG_TEGRA_AHB)		+= tegra-ahb.o
> diff --git a/drivers/platform/arm/tegra-ahb.c b/drivers/platform/arm/tegra-ahb.c
> new file mode 100644
> index 0000000..dcad2ec
> --- /dev/null
> +++ b/drivers/platform/arm/tegra-ahb.c
> @@ -0,0 +1,278 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + * Copyright (C) 2011 Google, Inc.
> + *
> + * Author:
> + *	Jay Cheng <jacheng@nvidia.com>
> + *	James Wylder <james.wylder@motorola.com>
> + *	Benoit Goby <benoit@android.com>
> + *	Colin Cross <ccross@android.com>
> + *	Hiroshi DOYU <hdoyu@nvidia.com>
> + *
> + * 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>
> +
> +#include <mach/iomap.h>

why do you need mach/iomap.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;
> +	void		*ctx;
> +	struct device	*dev;
> +};
> +
> +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);
> +	u32 *p = ahb->ctx;
> +
> +	for (i = 0; i < ARRAY_SIZE(tegra_ahb_gizmo); i++)
> +		p[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);
> +	u32 *p = ahb->ctx;
> +
> +	for (i = 0; i < ARRAY_SIZE(tegra_ahb_gizmo); i++)
> +		gizmo_writel(ahb, p[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;
> +	ahb->ctx = ahb + 1;
> +
> +	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)
> +{
> +	platform_set_drvdata(pdev, NULL);
> +	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_init(void)
> +{
> +	return platform_driver_register(&tegra_ahb_driver);
> +}
> +postcore_initcall(tegra_ahb_init);
> +
> +static void __exit tegra_ahb_exit(void)
> +{
> +	platform_driver_unregister(&tegra_ahb_driver);
> +}
> +module_exit(tegra_ahb_exit);
> +
> +MODULE_AUTHOR("Hiroshi DOYU <hdoyu@nvidia.com>");
> +MODULE_DESCRIPTION("Tegra AHB driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRV_NAME);
> -- 
> 1.7.5.4
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3359 bytes --]

Hi,

On Wed, Apr 25, 2012 at 02:07:37PM +0300, Hiroshi DOYU wrote:
> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> ready.
> 
> Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> Cc: Felipe Balbi <balbi@ti.com>
> ---
>  arch/arm/mach-tegra/include/mach/tegra-ahb.h |   19 +++++++++++++++++++
>  drivers/platform/arm/tegra-ahb.c             |   24 ++++++++++++++++++++++++
>  2 files changed, 43 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..296688c
> --- /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(void);
> +
> +#endif	/* __MACH_TEGRA_AHB_H__ */
> diff --git a/drivers/platform/arm/tegra-ahb.c b/drivers/platform/arm/tegra-ahb.c
> index dcad2ec..7ad9dfd 100644
> --- a/drivers/platform/arm/tegra-ahb.c
> +++ b/drivers/platform/arm/tegra-ahb.c
> @@ -78,6 +78,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,
> @@ -126,6 +130,26 @@ 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_enable_smmu(struct device *dev, void *data)
> +{
> +	u32 val;
> +	struct tegra_ahb *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;
> +}
> +
> +int tegra_ahb_enable_smmu(void)
> +{
> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> +				      __tegra_ahb_enable_smmu);
> +}
> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> +#endif

this is still not correct. If you will always call this whenever you run
on tegra 3, why do you even expose this function ? You can use your
compatible flag to do runtime detection of where you're running and call
this accordingly. Something like:

static const struct of_device_id tegra_ahb_match_table[] = {
{
	.compatible = "nvidia,tegra2",
}, {
	.compatible = "nvidia,tegra3",
	.data	= AHB_HAS_SMMU,
},
{  }
};

or something similar, then on probe, you can capture your id and check
if that bit is true, if it is just call tegra_ahb_enable_smmu(), you
really don't need all this trickery with driver_for_each_device().

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver

[Hiroshi Doyu]

From: Felipe Balbi <balbi@ti.com>
Subject: Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver
Date: Wed, 25 Apr 2012 13:25:20 +0200
Message-ID: <20120425112519.GB3564@arwen.pp.htv.fi>

> * PGP Signed by an unknown key
> 
> On Wed, Apr 25, 2012 at 02:07:36PM +0300, Hiroshi DOYU wrote:
> > The 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 <hdoyu@nvidia.com>
> > Cc: Felipe Balbi <balbi@ti.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > ---
> > v4:
> > Fixed the comments from Felipe/Russell.
> > This is now located under drivers/platform/arm.
> > v3:
> > Use platform_device to get info from dt dynamically.(Felipe/Arnd)
> >
> > Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
> 
> Just one thing, you seem to have missed Russell's comment about this
> driver being generic. You still add a TEGRA_AHB driver,

For generalization, there seems to be many parts in this driver
depending on SoC implementation, and I'm not so sure how much S/W
compatibility can be kept from H/W spec. Is there any other similar
SoC AHB driver in kernel?

> also, you don't need <mach/iomap.h>. Apart from those, you can add:

Ok, I'll fix and add your Reviewed-by:. Thank you for your review.

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

[Stephen Warren]

On 04/25/2012 05:29 AM, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Apr 25, 2012 at 02:07:37PM +0300, Hiroshi DOYU wrote:
>> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
>> ready.

>> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
>> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
>> +{
>> +	u32 val;
>> +	struct tegra_ahb *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;
>> +}
>> +
>> +int tegra_ahb_enable_smmu(void)
>> +{
>> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
>> +				      __tegra_ahb_enable_smmu);
>> +}
>> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
>> +#endif
> 
> this is still not correct. If you will always call this whenever you run
> on tegra 3, why do you even expose this function ?

I think the issue is that this function should only be called after the
SMMU driver has initialized the SMMU, and it's ready to be activated.

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

[Hiroshi Doyu]

On Wed, 25 Apr 2012 17:51:50 +0200
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 04/25/2012 05:29 AM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Wed, Apr 25, 2012 at 02:07:37PM +0300, Hiroshi DOYU wrote:
> >> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> >> ready.
> 
> >> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> >> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
> >> +{
> >> +	u32 val;
> >> +	struct tegra_ahb *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;
> >> +}
> >> +
> >> +int tegra_ahb_enable_smmu(void)
> >> +{
> >> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> >> +				      __tegra_ahb_enable_smmu);
> >> +}
> >> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> >> +#endif
> > 
> > this is still not correct. If you will always call this whenever you run
> > on tegra 3, why do you even expose this function ?
> 
> I think the issue is that this function should only be called after the
> SMMU driver has initialized the SMMU, and it's ready to be activated.

Right. "tegra_ahb_enable_smmu()" is not supposed to be called at
tegra_ahb_probe(), but it is always expected to be called after SMMU
is initialized/gets ready.

The sequence is:
 1, AHB is probed.
 ...<any other operations>...
 2, SMMU is probed.
 ...<any other operations>...
 3, SMMU is ready, then,
 4, SMMU lets AHB know that SMMU is ready to be used by AHB clients.

The above step 4 is "tegra_ahb_enable_smmu()".

Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver

[Stephen Warren]

On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> The 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.

I think this code looks reasonable. I'd like to see an ack from Russell,
Arnd, and Olof on the final location of the files though.

I note that MAINTAINERS doesn't have an entry for drivers/platform/
(except x86/ sub-dir) or drivers/platform/arm/.

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

[Stephen Warren]

On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> ready.

> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
...
> +int tegra_ahb_enable_smmu(void)
> +{
> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> +				      __tegra_ahb_enable_smmu);
> +}
> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> +#endif

That looks like a neat solution to avoid having a global device object.

However, if that driver_for_each_device finds no devices, the function
still succeeds. That doesn't seem right, and doesn't allow e.g. the SMMU
to defer its probe until the AHB driver has completed.

Perhaps add a local int variable to tegra_ahb_enable_smmu(), pass the
address to __tegra_ahb_enable_smmu, and have it increment the int. Then,
after calling driver_for_each_device,:

if (!ahb_device_count)
    return -EPROBE_DEFER
if (WARN_ON(ahb_device_count != 1))
    return -EINVAL;
return 0;

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

[Stephen Warren]

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

> @@ -397,11 +384,7 @@ static void smmu_setup_regs(struct smmu_device *smmu)
...
> +	tegra_ahb_enable_smmu();

You need to check for an error there, and make smmu_setup_regs()
propagate any error back to its callers and check it there too.

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

[Stephen Warren]

On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> Add AHB entry for tegra20/30.

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

> +	ahb: ahb@6000c004 {
> +		compatible = "nvidia,tegra30-ahb", "nvidia,tegra20-ahb";

This should only be tegra20-ahb in the Tegra20 file.

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]

Hi,

On Thu, Apr 26, 2012 at 01:55:13PM -0600, Stephen Warren wrote:
> On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> > Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> > ready.
> 
> > +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> > +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
> ...
> > +int tegra_ahb_enable_smmu(void)
> > +{
> > +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> > +				      __tegra_ahb_enable_smmu);
> > +}
> > +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> > +#endif
> 
> That looks like a neat solution to avoid having a global device object.

except that it won't work always. If you happen to have two AHB bridges,
each using a separate smmu but only one smmu is ready, this will set
SMMU_INIT_DONE on both bridges.

> However, if that driver_for_each_device finds no devices, the function
> still succeeds. That doesn't seem right, and doesn't allow e.g. the SMMU
> to defer its probe until the AHB driver has completed.
> 
> Perhaps add a local int variable to tegra_ahb_enable_smmu(), pass the
> address to __tegra_ahb_enable_smmu, and have it increment the int. Then,
> after calling driver_for_each_device,:
> 
> if (!ahb_device_count)
>     return -EPROBE_DEFER
> if (WARN_ON(ahb_device_count != 1))
>     return -EINVAL;
> return 0;

that would look, well, weird. Why don't you just different initcall
leves for this ? Maybe smmu goes into postcore_initcall() and tegra_ahb
goes into postcore_initcall_sync() ?? then you know that SMMU will be
ready by the time you call tegra_ahb probe. Well, unless smmu's probe
fail, but then again, IIUC it won't work anyway...

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Stephen Warren]

On 04/26/2012 02:26 PM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Apr 26, 2012 at 01:55:13PM -0600, Stephen Warren wrote:
>> On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
>>> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
>>> ready.
>>
>>> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
>>> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
>> ...
>>> +int tegra_ahb_enable_smmu(void)
>>> +{
>>> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
>>> +				      __tegra_ahb_enable_smmu);
>>> +}
>>> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
>>> +#endif
>>
>> That looks like a neat solution to avoid having a global device object.
> 
> except that it won't work always. If you happen to have two AHB bridges,
> each using a separate smmu but only one smmu is ready, this will set
> SMMU_INIT_DONE on both bridges.

There is only 1.

>> However, if that driver_for_each_device finds no devices, the function
>> still succeeds. That doesn't seem right, and doesn't allow e.g. the SMMU
>> to defer its probe until the AHB driver has completed.
>>
>> Perhaps add a local int variable to tegra_ahb_enable_smmu(), pass the
>> address to __tegra_ahb_enable_smmu, and have it increment the int. Then,
>> after calling driver_for_each_device,:
>>
>> if (!ahb_device_count)
>>     return -EPROBE_DEFER
>> if (WARN_ON(ahb_device_count != 1))
>>     return -EINVAL;
>> return 0;
> 
> that would look, well, weird. Why don't you just different initcall
> leves for this ? Maybe smmu goes into postcore_initcall() and tegra_ahb
> goes into postcore_initcall_sync() ?? then you know that SMMU will be
> ready by the time you call tegra_ahb probe. Well, unless smmu's probe
> fail, but then again, IIUC it won't work anyway...

Uggh. I'd rather all these devices just got instantiated from device
tree and relied on deferred probe to manage any ordering, rather than
playing complex games with multiple initcall levels (and in the end
probably having to invent more and more initcall levels to correctly
represent all the dependencies).

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2792 bytes --]

Hi,

On Thu, Apr 26, 2012 at 02:32:56PM -0600, Stephen Warren wrote:
> On 04/26/2012 02:26 PM, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Apr 26, 2012 at 01:55:13PM -0600, Stephen Warren wrote:
> >> On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> >>> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> >>> ready.
> >>
> >>> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> >>> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
> >> ...
> >>> +int tegra_ahb_enable_smmu(void)
> >>> +{
> >>> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> >>> +				      __tegra_ahb_enable_smmu);
> >>> +}
> >>> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> >>> +#endif
> >>
> >> That looks like a neat solution to avoid having a global device object.
> > 
> > except that it won't work always. If you happen to have two AHB bridges,
> > each using a separate smmu but only one smmu is ready, this will set
> > SMMU_INIT_DONE on both bridges.
> 
> There is only 1.

that's why there's a "if you happen to have" statement. If you stick to
this "there is only 1" argument, why do you even make this into a
platform driver ? Just stick the entire code hidden on the
machine_init() code. Drivers a supposed to be able to instantiated
multiple times and always work, this method won't work if tegra99999
ends up with two AHB bridges/SMMUs

> >> However, if that driver_for_each_device finds no devices, the function
> >> still succeeds. That doesn't seem right, and doesn't allow e.g. the SMMU
> >> to defer its probe until the AHB driver has completed.
> >>
> >> Perhaps add a local int variable to tegra_ahb_enable_smmu(), pass the
> >> address to __tegra_ahb_enable_smmu, and have it increment the int. Then,
> >> after calling driver_for_each_device,:
> >>
> >> if (!ahb_device_count)
> >>     return -EPROBE_DEFER
> >> if (WARN_ON(ahb_device_count != 1))
> >>     return -EINVAL;
> >> return 0;
> > 
> > that would look, well, weird. Why don't you just different initcall
> > leves for this ? Maybe smmu goes into postcore_initcall() and tegra_ahb
> > goes into postcore_initcall_sync() ?? then you know that SMMU will be
> > ready by the time you call tegra_ahb probe. Well, unless smmu's probe
> > fail, but then again, IIUC it won't work anyway...
> 
> Uggh. I'd rather all these devices just got instantiated from device
> tree and relied on deferred probe to manage any ordering, rather than
> playing complex games with multiple initcall levels (and in the end
> probably having to invent more and more initcall levels to correctly
> represent all the dependencies).

then do that... it'll be better than current trickery with
driver_for_each_device() and my initcall trickery ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver

[Russell King - ARM Linux]

On Thu, Apr 26, 2012 at 01:51:28PM -0600, Stephen Warren wrote:
> On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> > The 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.
> 
> I think this code looks reasonable. I'd like to see an ack from Russell,
> Arnd, and Olof on the final location of the files though.

Well, the big question which needs answering is whether this AHB software
interface is something specific to Tegra or whether it really is something
generic.  There's been some hints in the previous thread that it's
specific to Tegra, so it may not after all make sense for it to be a
generic driver.

However, we _have_ decided "no more drivers under arch/arm".  So I really
don't want to see "struct xxx_driver" appearing in any code under that
subdirectory or one of its decendents.  I don't have much more to say
about location than that.

Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver

[Hiroshi Doyu]

On Thu, 26 Apr 2012 23:59:03 +0200
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Thu, Apr 26, 2012 at 01:51:28PM -0600, Stephen Warren wrote:
> > On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> > > The 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.
> > 
> > I think this code looks reasonable. I'd like to see an ack from Russell,
> > Arnd, and Olof on the final location of the files though.
> 
> Well, the big question which needs answering is whether this AHB software
> interface is something specific to Tegra or whether it really is something
> generic.  There's been some hints in the previous thread that it's
> specific to Tegra, so it may not after all make sense for it to be a
> generic driver.
> 
> However, we _have_ decided "no more drivers under arch/arm".  So I really
> don't want to see "struct xxx_driver" appearing in any code under that
> subdirectory or one of its decendents.  I don't have much more to say
> about location than that.

What about having this driver under "drivers/amba"?

If other similiar drivers are coming up, it's easy to find rather than
having this arch/arm/mach-*. There may be some possibility of
generalization later, then. Also it meets the requirement of no
"struct xxxx_driver" under arch/arm. Arnd?

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

[Hiroshi Doyu]

From: Felipe Balbi <balbi@ti.com>
Subject: Re: [PATCHv3 2/4] ARM: tegra: Add SMMU enabler in AHB
Date: Thu, 26 Apr 2012 22:38:48 +0200
Message-ID: <20120426203847.GC30690@arwen.pp.htv.fi>

> * PGP Signed by an unknown key
> 
> Hi,
> 
> On Thu, Apr 26, 2012 at 02:32:56PM -0600, Stephen Warren wrote:
> > On 04/26/2012 02:26 PM, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Thu, Apr 26, 2012 at 01:55:13PM -0600, Stephen Warren wrote:
> > >> On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> > >>> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> > >>> ready.
> > >>
> > >>> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> > >>> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
> > >> ...
> > >>> +int tegra_ahb_enable_smmu(void)
> > >>> +{
> > >>> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> > >>> +				      __tegra_ahb_enable_smmu);
> > >>> +}
> > >>> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> > >>> +#endif
> > >>
> > >> That looks like a neat solution to avoid having a global device object.
> > > 
> > > except that it won't work always. If you happen to have two AHB bridges,
> > > each using a separate smmu but only one smmu is ready, this will set
> > > SMMU_INIT_DONE on both bridges.
> > 
> > There is only 1.
> 
> that's why there's a "if you happen to have" statement. If you stick to
> this "there is only 1" argument, why do you even make this into a
> platform driver ? Just stick the entire code hidden on the
> machine_init() code. Drivers a supposed to be able to instantiated
> multiple times and always work, this method won't work if tegra99999
> ends up with two AHB bridges/SMMUs
> 
> > >> However, if that driver_for_each_device finds no devices, the function
> > >> still succeeds. That doesn't seem right, and doesn't allow e.g. the SMMU
> > >> to defer its probe until the AHB driver has completed.
> > >>
> > >> Perhaps add a local int variable to tegra_ahb_enable_smmu(), pass the
> > >> address to __tegra_ahb_enable_smmu, and have it increment the int. Then,
> > >> after calling driver_for_each_device,:
> > >>
> > >> if (!ahb_device_count)
> > >>     return -EPROBE_DEFER
> > >> if (WARN_ON(ahb_device_count != 1))
> > >>     return -EINVAL;
> > >> return 0;
> > > 
> > > that would look, well, weird. Why don't you just different initcall
> > > leves for this ? Maybe smmu goes into postcore_initcall() and tegra_ahb
> > > goes into postcore_initcall_sync() ?? then you know that SMMU will be
> > > ready by the time you call tegra_ahb probe. Well, unless smmu's probe
> > > fail, but then again, IIUC it won't work anyway...
> > 
> > Uggh. I'd rather all these devices just got instantiated from device
> > tree and relied on deferred probe to manage any ordering, rather than
> > playing complex games with multiple initcall levels (and in the end
> > probably having to invent more and more initcall levels to correctly
> > represent all the dependencies).
> 
> then do that... it'll be better than current trickery with
> driver_for_each_device() and my initcall trickery ;-)

Then, what about something like the following?

 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 *ahb)
 {
 	struct device *dev;
 	u32 val;
 
 	dev = driver_find_device(&tegra_ahb_driver.driver, NULL, ahb,
 				 tegra_ahb_match_by_smmu);
 	if (!dev)
 		return -ENODEV;
 
 	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);

	Modified arch/arm/boot/dts/tegra30.dtsi
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 655bc47..28f9138 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -242,5 +242,6 @@
 	smmu {
 		compatible = "nvidia,tegra30-smmu";
 		mc = <&mc>;
+		ahb = <&ahb>;
 	};
 };

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

[Hiroshi Doyu]

From: Hiroshi DOYU <hdoyu@nvidia.com>
Subject: Re: [PATCHv3 2/4] ARM: tegra: Add SMMU enabler in AHB
Date: Fri, 27 Apr 2012 09:48:26 +0300 (EEST)
Message-ID: <20120427.094826.1181797260264746303.hdoyu@nvidia.com>

> From: Felipe Balbi <balbi@ti.com>
> Subject: Re: [PATCHv3 2/4] ARM: tegra: Add SMMU enabler in AHB
> Date: Thu, 26 Apr 2012 22:38:48 +0200
> Message-ID: <20120426203847.GC30690@arwen.pp.htv.fi>
> 
> > * PGP Signed by an unknown key
> > 
> > Hi,
> > 
> > On Thu, Apr 26, 2012 at 02:32:56PM -0600, Stephen Warren wrote:
> > > On 04/26/2012 02:26 PM, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Thu, Apr 26, 2012 at 01:55:13PM -0600, Stephen Warren wrote:
> > > >> On 04/25/2012 05:07 AM, Hiroshi DOYU wrote:
> > > >>> Add extern func, "tegra_ahb_enable_smmu()" to inform AHB that SMMU is
> > > >>> ready.
> > > >>
> > > >>> +#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> > > >>> +static int __tegra_ahb_enable_smmu(struct device *dev, void *data)
> > > >> ...
> > > >>> +int tegra_ahb_enable_smmu(void)
> > > >>> +{
> > > >>> +	return driver_for_each_device(&tegra_ahb_driver.driver, NULL, NULL,
> > > >>> +				      __tegra_ahb_enable_smmu);
> > > >>> +}
> > > >>> +EXPORT_SYMBOL(tegra_ahb_enable_smmu);
> > > >>> +#endif
> > > >>
> > > >> That looks like a neat solution to avoid having a global device object.
> > > > 
> > > > except that it won't work always. If you happen to have two AHB bridges,
> > > > each using a separate smmu but only one smmu is ready, this will set
> > > > SMMU_INIT_DONE on both bridges.
> > > 
> > > There is only 1.
> > 
> > that's why there's a "if you happen to have" statement. If you stick to
> > this "there is only 1" argument, why do you even make this into a
> > platform driver ? Just stick the entire code hidden on the
> > machine_init() code. Drivers a supposed to be able to instantiated
> > multiple times and always work, this method won't work if tegra99999
> > ends up with two AHB bridges/SMMUs
> > 
> > > >> However, if that driver_for_each_device finds no devices, the function
> > > >> still succeeds. That doesn't seem right, and doesn't allow e.g. the SMMU
> > > >> to defer its probe until the AHB driver has completed.
> > > >>
> > > >> Perhaps add a local int variable to tegra_ahb_enable_smmu(), pass the
> > > >> address to __tegra_ahb_enable_smmu, and have it increment the int. Then,
> > > >> after calling driver_for_each_device,:
> > > >>
> > > >> if (!ahb_device_count)
> > > >>     return -EPROBE_DEFER
> > > >> if (WARN_ON(ahb_device_count != 1))
> > > >>     return -EINVAL;
> > > >> return 0;
> > > > 
> > > > that would look, well, weird. Why don't you just different initcall
> > > > leves for this ? Maybe smmu goes into postcore_initcall() and tegra_ahb
> > > > goes into postcore_initcall_sync() ?? then you know that SMMU will be
> > > > ready by the time you call tegra_ahb probe. Well, unless smmu's probe
> > > > fail, but then again, IIUC it won't work anyway...
> > > 
> > > Uggh. I'd rather all these devices just got instantiated from device
> > > tree and relied on deferred probe to manage any ordering, rather than
> > > playing complex games with multiple initcall levels (and in the end
> > > probably having to invent more and more initcall levels to correctly
> > > represent all the dependencies).
> > 
> > then do that... it'll be better than current trickery with
> > driver_for_each_device() and my initcall trickery ;-)
> 
> Then, what about something like the following?
> 
>  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 *ahb)
>  {
>  	struct device *dev;
>  	u32 val;
>  
>  	dev = driver_find_device(&tegra_ahb_driver.driver, NULL, ahb,
>  				 tegra_ahb_match_by_smmu);
>  	if (!dev)
>  		return -ENODEV;

+		return -EPROBE_DEFER

Maybe this is better(?).

>  
>  	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);
> 
> 	Modified arch/arm/boot/dts/tegra30.dtsi
> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
> index 655bc47..28f9138 100644
> --- a/arch/arm/boot/dts/tegra30.dtsi
> +++ b/arch/arm/boot/dts/tegra30.dtsi
> @@ -242,5 +242,6 @@
>  	smmu {
>  		compatible = "nvidia,tegra30-smmu";
>  		mc = <&mc>;
> +		ahb = <&ahb>;
>  	};
>  };

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

[Stephen Warren]

On 04/27/2012 12:48 AM, Hiroshi Doyu wrote:
...
> Then, what about something like the following?
> 
>  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
>  }

I imagine that function and the driver_find_device() call that uses it
will be pretty common. It may make sense as a drivers/of utility
function, although perhaps we can wait until later to refactor it, once
we actually have multiple pieces of code that do this same thing.

>  int tegra_ahb_enable_smmu(struct device_node *ahb)
>  {
>  	struct device *dev;
>  	u32 val;
>  
>  	dev = driver_find_device(&tegra_ahb_driver.driver, NULL, ahb,
>  				 tegra_ahb_match_by_smmu);
>  	if (!dev)
>  		return -ENODEV;

Yes, -EPROBE_DEFER here as you mentioned in your followup email.

>  
>  	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);
> 
> 	Modified arch/arm/boot/dts/tegra30.dtsi
> diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
> index 655bc47..28f9138 100644
> --- a/arch/arm/boot/dts/tegra30.dtsi
> +++ b/arch/arm/boot/dts/tegra30.dtsi
> @@ -242,5 +242,6 @@
>  	smmu {
>  		compatible = "nvidia,tegra30-smmu";
>  		mc = <&mc>;
> +		ahb = <&ahb>;
>  	};
>  };

Yes, that's probably the best (even only) way to connect the two drivers
precisely.

Re: [PATCHv3 1/4] ARM: tegra: Add AHB driver

[Arnd Bergmann]

On Friday 27 April 2012, Hiroshi Doyu wrote:
> What about having this driver under "drivers/amba"?
> 
> If other similiar drivers are coming up, it's easy to find rather than
> having this arch/arm/mach-*. There may be some possibility of
> generalization later, then. Also it meets the requirement of no
> "struct xxxx_driver" under arch/arm. Arnd?

I'm fine with either drivers/amba (arguing that it's for a specific
amba bus even if it's highly platform specific) or arch/arm/mach-tegra
(arguing that it's platform code and not actually a device driver even
if it appears like one). If Russell doesn't mind having it in
drivers/amba, I'd say we should go for that.

	Arnd

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

[Stephen Warren]

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 <hdoyu@nvidia.com>
> Cc: Felipe Balbi <balbi@ti.com>

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

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

[Hiroshi DOYU]

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

Signed-off-by: Hiroshi DOYU <hdoyu@nvidia.com>
Cc: Felipe Balbi <balbi@ti.com>
---
 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