From: Stephen Warren <swarren@xxxxxxxxxxxxx>
Subject: Re: [PATCH v2] ARM: dt: tegra20: Add GART device
Date: Mon, 7 May 2012 18:06:46 +0200
Message-ID: <4FA7F316.7070904@xxxxxxxxxxxxx>
> On 05/07/2012 05:47 AM, Hiroshi Doyu wrote:
> > Stephen Warren wrote at Fri, 4 May 2012 19:24:45 +0200:
> ...
> >> 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>;
> >> };
> ...
> > 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...
>
> > diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>
> > +#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
> > +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.
> > static int tegra_smmu_probe(struct platform_device *pdev)
>
> > + 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.
> > + 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
