From: Stephen Warren <redacted>
Subject: Re: [PATCH v2] ARM: dt: tegra20: Add GART device
Date: Mon, 7 May 2012 18:06:46 +0200
Message-ID: [off-list ref]

On 05/07/2012 05:47 AM, Hiroshi Doyu wrote:

quoted

Stephen Warren wrote at Fri, 4 May 2012 19:24:45 +0200:

...

quoted

quoted

Perhaps the simplest way is to represent those chunks as separate reg
entries:

    mc@7000f010 {
        compatible = "nvidia,tegra30-mc";
        reg = <0x7000f010 0x1d4 0x70000228 0xd4>;
    };

    smmu@0x70000050 {
        compatible = "nvidia,tegra30-smmu";
        reg = <0x70000010 0x2c 0x70000228 0x5c>;
    };

...

quoted

Ok, let's start with the simplest way, and improve later with nice
framework.

Uggh. So that patch is for the SMMU, but this thread is about the GART.
Also, attaching patches makes it more difficult to review them, since
them may not appear inline when reading the mail, and certainly don't
when replying. Anyway...

quoted

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

quoted

+#include <mach/tegra-ahb.h>

Note that this patch therefor depends on the AHB series that you just
posted, and isn't applied anywhere yet.

Right.

FYI: the AHB series:
http://article.gmane.org/gmane.linux.ports.tegra/4657

quoted

+static int get_reg_bank(u32 offset)
 {
-	writel(val, smmu->regs + offs);
+	int i;
+	const struct smmu_reg_bank {
+		u32 start;
+		size_t size;
+	} r[] = {
+		{
+			.start = 0x10,
+			.size = 0x2c,
+		},
+		{
+			.start = 0x1f0,
+			.size = 0x10,
+		},
+		{
+			.start = 0x228,
+			.size = 0x5c,
+		},
+	};
+
+	for (i = 0; i < ARRAY_SIZE(r); i++) {
+		BUG_ON(offset < r[i].start);
+		if (offset < r[i].start + r[i].size)
+			return i;
+	}
+	BUG();
+	return 0; /* Never reach here */
 }

While stylistically nice, I'm not sure that will be as likely to
optimize out as the simple if/else code I gave as an example. I suppose
it's not a big deal though.

Ok, you meant compiler optimization. I'll compare their output and may
use the original(yours) if the original is better.

quoted

 static int tegra_smmu_probe(struct platform_device *pdev)

quoted

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

So this also relies on the generic DMA window parsing patch too. I don't
think that's checked in anywhere yet right?

Yes. I'll ask the feedback on that patch.

quoted

+	size >>= SMMU_PAGE_SHIFT;
+	if (!size)
+		return -ENODEV;

It seems like this should validate that !(size & ((1 << SMMU_PAGE_SHIFT)
- 1)) too?

Yes

This patch looks like it adds the full implementation of device tree
support, yet doesn't add any documentation for the binding.

I'll add the doc.

Finally, I assume you're going to post a similar patch for the Tegra20
GART, so its register ranges don't overlap the Tegra20 MC?

Right.