* [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <510800F7.7020507-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-01-29 17:56 ` Hiroshi Doyu
[not found] ` <1359482169-26756-1-git-send-email-hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Hiroshi Doyu @ 2013-01-29 17:56 UTC (permalink / raw)
To: joro-zLv9SwRftAIdnm+yROfE0A, swarren-3lzwWm7+Weoh9ZMKESR00Q
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Presently SMMU registers are located in discontiguous 3 blocks. They
are interleaved by MC registers. Ideally SMMU register blocks should
be in an independent one block, but it is too late to change this H/W
design. In the future Tegra chips over some generations, it is
expected that some of register block "size" can be extended towards
the end and also more new register blocks will be added at most a few
blocks. The starting address of each existing block won't change. This
patch allocates multiple number of register blocks dynamically based
on the info passed from DT. Those ranges are verified in the
accessors{read,write}. This may sacrifice some performance because a
new accessors prevents compiler optimization of a fixed size register
offset calculation. Since SMMU register accesses are not so frequent,
this would be acceptable. This patch is necessary to unify
"tegra-smmu.ko" over some Tegra SoC generations.
Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v3:
Fixed overlapped register validation. (Stephen Warren)
Fixed typo s/regiter/register/. (Stephen Warren)
---
drivers/iommu/tegra-smmu.c | 61 ++++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index fc17889..d1448f9 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -293,7 +293,11 @@ struct smmu_debugfs_info {
* Per SMMU device - IOMMU device
*/
struct smmu_device {
- void __iomem *regs[NUM_SMMU_REG_BANKS];
+ void __iomem *regbase; /* register offset base */
+ void __iomem **regs; /* register block start address array */
+ void __iomem **rege; /* register block end address array */
+ int nregs; /* number of register blocks */
+
unsigned long iovmm_base; /* remappable base address */
unsigned long page_count; /* total remappable size */
spinlock_t lock;
@@ -325,35 +329,33 @@ static struct smmu_device *smmu_handle; /* unique for a system */
*/
static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
{
- BUG_ON(offs < 0x10);
- if (offs < 0x3c)
- return readl(smmu->regs[0] + offs - 0x10);
- BUG_ON(offs < 0x1f0);
- if (offs < 0x200)
- return readl(smmu->regs[1] + offs - 0x1f0);
- BUG_ON(offs < 0x228);
- if (offs < 0x284)
- return readl(smmu->regs[2] + offs - 0x228);
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(addr < smmu->regs[i]);
+ if (addr < smmu->rege[i])
+ return readl(addr);
+ }
+
BUG();
}
static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
{
- BUG_ON(offs < 0x10);
- if (offs < 0x3c) {
- writel(val, smmu->regs[0] + offs - 0x10);
- return;
- }
- BUG_ON(offs < 0x1f0);
- if (offs < 0x200) {
- writel(val, smmu->regs[1] + offs - 0x1f0);
- return;
- }
- BUG_ON(offs < 0x228);
- if (offs < 0x284) {
- writel(val, smmu->regs[2] + offs - 0x228);
- return;
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(addr < smmu->regs[i]);
+ if (addr < smmu->rege[i]) {
+ writel(val, addr);
+ return;
+ }
}
+
BUG();
}
@@ -1170,7 +1172,13 @@ static int tegra_smmu_probe(struct platform_device *pdev)
return -ENOMEM;
}
- for (i = 0; i < ARRAY_SIZE(smmu->regs); i++) {
+ smmu->nregs = pdev->num_resources;
+ smmu->regs = devm_kzalloc(dev, 2 * smmu->nregs * sizeof(*smmu->regs),
+ GFP_KERNEL);
+ smmu->rege = smmu->regs + smmu->nregs;
+ if (!smmu->regs)
+ return -ENOMEM;
+ for (i = 0; i < smmu->nregs; i++) {
struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
@@ -1179,7 +1187,10 @@ static int tegra_smmu_probe(struct platform_device *pdev)
smmu->regs[i] = devm_request_and_ioremap(&pdev->dev, res);
if (!smmu->regs[i])
return -EBUSY;
+ smmu->rege[i] = smmu->regs[i] + resource_size(res);
}
+ /* Same as "mc" 1st register block start address */
+ smmu->regbase = (void __iomem *)((u32)smmu->regs[0] & ~PAGE_MASK);
err = of_get_dma_window(dev->of_node, NULL, 0, NULL, &base, &size);
if (err)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <1359482169-26756-1-git-send-email-hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-01-29 17:58 ` Stephen Warren
0 siblings, 0 replies; 9+ messages in thread
From: Stephen Warren @ 2013-01-29 17:58 UTC (permalink / raw)
To: Hiroshi Doyu
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
On 01/29/2013 10:56 AM, Hiroshi Doyu wrote:
> Presently SMMU registers are located in discontiguous 3 blocks. They
> are interleaved by MC registers. Ideally SMMU register blocks should
> be in an independent one block, but it is too late to change this H/W
> design. In the future Tegra chips over some generations, it is
> expected that some of register block "size" can be extended towards
> the end and also more new register blocks will be added at most a few
> blocks. The starting address of each existing block won't change. This
> patch allocates multiple number of register blocks dynamically based
> on the info passed from DT. Those ranges are verified in the
> accessors{read,write}. This may sacrifice some performance because a
> new accessors prevents compiler optimization of a fixed size register
> offset calculation. Since SMMU register accesses are not so frequent,
> this would be acceptable. This patch is necessary to unify
> "tegra-smmu.ko" over some Tegra SoC generations.
Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
@ 2013-01-31 8:14 Hiroshi Doyu
[not found] ` <1359620050-28727-1-git-send-email-hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Hiroshi Doyu @ 2013-01-31 8:14 UTC (permalink / raw)
To: joro-zLv9SwRftAIdnm+yROfE0A
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, Hiroshi Doyu
Presently SMMU registers are located in discontiguous 3 blocks. They
are interleaved by MC registers. Ideally SMMU register blocks should
be in an independent one block, but it is too late to change this H/W
design. In the future Tegra chips over some generations, it is
expected that some of register block "size" can be extended towards
the end and also more new register blocks will be added at most a few
blocks. The starting address of each existing block won't change. This
patch allocates multiple number of register blocks dynamically based
on the info passed from DT. Those ranges are verified in the
accessors{read,write}. This may sacrifice some performance because a
new accessors prevents compiler optimization of a fixed size register
offset calculation. Since SMMU register accesses are not so frequent,
this would be acceptable. This patch is necessary to unify
"tegra-smmu.ko" over some Tegra SoC generations.
Signed-off-by: Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
drivers/iommu/tegra-smmu.c | 61 ++++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 25c1210..024a7e1 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -293,7 +293,11 @@ struct smmu_debugfs_info {
* Per SMMU device - IOMMU device
*/
struct smmu_device {
- void __iomem *regs[NUM_SMMU_REG_BANKS];
+ void __iomem *regbase; /* register offset base */
+ void __iomem **regs; /* register block start address array */
+ void __iomem **rege; /* register block end address array */
+ int nregs; /* number of register blocks */
+
unsigned long iovmm_base; /* remappable base address */
unsigned long page_count; /* total remappable size */
spinlock_t lock;
@@ -325,35 +329,33 @@ static struct smmu_device *smmu_handle; /* unique for a system */
*/
static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
{
- BUG_ON(offs < 0x10);
- if (offs < 0x3c)
- return readl(smmu->regs[0] + offs - 0x10);
- BUG_ON(offs < 0x1f0);
- if (offs < 0x200)
- return readl(smmu->regs[1] + offs - 0x1f0);
- BUG_ON(offs < 0x228);
- if (offs < 0x284)
- return readl(smmu->regs[2] + offs - 0x228);
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(addr < smmu->regs[i]);
+ if (addr <= smmu->rege[i])
+ return readl(addr);
+ }
+
BUG();
}
static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
{
- BUG_ON(offs < 0x10);
- if (offs < 0x3c) {
- writel(val, smmu->regs[0] + offs - 0x10);
- return;
- }
- BUG_ON(offs < 0x1f0);
- if (offs < 0x200) {
- writel(val, smmu->regs[1] + offs - 0x1f0);
- return;
- }
- BUG_ON(offs < 0x228);
- if (offs < 0x284) {
- writel(val, smmu->regs[2] + offs - 0x228);
- return;
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(addr < smmu->regs[i]);
+ if (addr <= smmu->rege[i]) {
+ writel(val, addr);
+ return;
+ }
}
+
BUG();
}
@@ -1170,7 +1172,13 @@ static int tegra_smmu_probe(struct platform_device *pdev)
return -ENOMEM;
}
- for (i = 0; i < ARRAY_SIZE(smmu->regs); i++) {
+ smmu->nregs = pdev->num_resources;
+ smmu->regs = devm_kzalloc(dev, 2 * smmu->nregs * sizeof(*smmu->regs),
+ GFP_KERNEL);
+ smmu->rege = smmu->regs + smmu->nregs;
+ if (!smmu->regs)
+ return -ENOMEM;
+ for (i = 0; i < smmu->nregs; i++) {
struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
@@ -1179,7 +1187,10 @@ static int tegra_smmu_probe(struct platform_device *pdev)
smmu->regs[i] = devm_request_and_ioremap(&pdev->dev, res);
if (!smmu->regs[i])
return -EBUSY;
+ smmu->rege[i] = smmu->regs[i] + resource_size(res);
}
+ /* Same as "mc" 1st regiter block start address */
+ smmu->regbase = (void __iomem *)((u32)smmu->regs[0] & ~PAGE_MASK);
err = of_get_dma_window(dev->of_node, NULL, 0, NULL, &base, &size);
if (err)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <1359620050-28727-1-git-send-email-hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-04 19:53 ` Joerg Roedel
[not found] ` <20130204195232.GA15278-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Joerg Roedel @ 2013-02-04 19:53 UTC (permalink / raw)
To: Hiroshi Doyu
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
On Thu, Jan 31, 2013 at 10:14:10AM +0200, Hiroshi Doyu wrote:
> drivers/iommu/tegra-smmu.c | 61 ++++++++++++++++++++++++++------------------
> 1 file changed, 36 insertions(+), 25 deletions(-)
Okay, applied this patch to arm/tegra, but
> static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
> {
> - BUG_ON(offs < 0x10);
> - if (offs < 0x3c)
> - return readl(smmu->regs[0] + offs - 0x10);
> - BUG_ON(offs < 0x1f0);
> - if (offs < 0x200)
> - return readl(smmu->regs[1] + offs - 0x1f0);
> - BUG_ON(offs < 0x228);
> - if (offs < 0x284)
> - return readl(smmu->regs[2] + offs - 0x228);
> + int i;
> +
> + for (i = 0; i < smmu->nregs; i++) {
> + void __iomem *addr = smmu->regbase + offs;
> +
> + BUG_ON(addr < smmu->regs[i]);
> + if (addr <= smmu->rege[i])
> + return readl(addr);
> + }
This loop is purely for checking offset to be valid. And this loop is
repeated in the smmu_write() function. I queued a patch on-top to make
this more clear. Please double-check:
>From 08c4ae0b8955bd818bdfd438a684bd5f8db7fb67 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
Date: Mon, 4 Feb 2013 20:40:58 +0100
Subject: [PATCH 1/1] iommu/tegra: smmu: Use helper function to check for
valid register offset
Do not repeat the checking loop in the read and write
functions. Use a single helper function for that check and
call it in both accessors.
Signed-off-by: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
---
drivers/iommu/tegra-smmu.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 3e07e8b..5ea8d66 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -327,36 +327,37 @@ static struct smmu_device *smmu_handle; /* unique for a system */
/*
* SMMU register accessors
*/
-static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
+static bool inline smmu_valid_reg(struct smmu_device *smmu,
+ void __iomem *addr)
{
int i;
for (i = 0; i < smmu->nregs; i++) {
- void __iomem *addr = smmu->regbase + offs;
-
- BUG_ON(addr < smmu->regs[i]);
+ if (addr < smmu->regs[i])
+ break;
if (addr <= smmu->rege[i])
- return readl(addr);
+ return true;
}
- BUG();
+ return false;
}
-static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
+static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
{
- int i;
+ void __iomem *addr = smmu->regbase + offs;
- for (i = 0; i < smmu->nregs; i++) {
- void __iomem *addr = smmu->regbase + offs;
+ BUG_ON(!smmu_valid_reg(smmu, addr));
- BUG_ON(addr < smmu->regs[i]);
- if (addr <= smmu->rege[i]) {
- writel(val, addr);
- return;
- }
- }
+ return readl(addr);
+}
+
+static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
+{
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(!smmu_valid_reg(smmu, addr));
- BUG();
+ writel(val, addr);
}
#define VA_PAGE_TO_PA(va, page) \
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <20130204195232.GA15278-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
@ 2013-02-04 20:31 ` Hiroshi Doyu
[not found] ` <20130204.223114.121259250064618894.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Hiroshi Doyu @ 2013-02-04 20:31 UTC (permalink / raw)
To: joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org, Stephen Warren
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hi Joerg,
Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote @ Mon, 4 Feb 2013 20:53:32 +0100:
> > static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
> > {
> > - BUG_ON(offs < 0x10);
> > - if (offs < 0x3c)
> > - return readl(smmu->regs[0] + offs - 0x10);
> > - BUG_ON(offs < 0x1f0);
> > - if (offs < 0x200)
> > - return readl(smmu->regs[1] + offs - 0x1f0);
> > - BUG_ON(offs < 0x228);
> > - if (offs < 0x284)
> > - return readl(smmu->regs[2] + offs - 0x228);
> > + int i;
> > +
> > + for (i = 0; i < smmu->nregs; i++) {
> > + void __iomem *addr = smmu->regbase + offs;
> > +
> > + BUG_ON(addr < smmu->regs[i]);
> > + if (addr <= smmu->rege[i])
> > + return readl(addr);
> > + }
>
> This loop is purely for checking offset to be valid. And this loop is
> repeated in the smmu_write() function. I queued a patch on-top to make
> this more clear. Please double-check:
Actually I did the similar thing in the first version of this patch(*1)
+static inline void smmu_check_reg_range(size_t offs)
+{
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ BUG_ON(offs < smmu->regs[i] - smmu->regbase);
+ if (offs <= smmu->rege[i] - smmu->regbase)
+ break;
+ }
+}
*1:
http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005072.html
Then, Stehpen pointed out about this check function(*2).
"... here, you'd be doing the loop every access anyway, so you may as
well not calculate regbase at all, move the body of
smmu_check_reg_range() into smmu_read()/smmu_write(), and do the access
inside the if statement inside the loop, with the per-range mapping."
*2:
http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005074.html
I might not get Stehpen's point in the latest patch(?), though....
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <20130204.223114.121259250064618894.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-04 20:35 ` Stephen Warren
[not found] ` <51101BA1.5090208-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2013-02-04 20:35 UTC (permalink / raw)
To: Hiroshi Doyu
Cc: joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org, Stephen Warren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On 02/04/2013 01:31 PM, Hiroshi Doyu wrote:
> Hi Joerg,
>
> Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org> wrote @ Mon, 4 Feb 2013 20:53:32 +0100:
>
>>> static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
>>> {
>>> - BUG_ON(offs < 0x10);
>>> - if (offs < 0x3c)
>>> - return readl(smmu->regs[0] + offs - 0x10);
>>> - BUG_ON(offs < 0x1f0);
>>> - if (offs < 0x200)
>>> - return readl(smmu->regs[1] + offs - 0x1f0);
>>> - BUG_ON(offs < 0x228);
>>> - if (offs < 0x284)
>>> - return readl(smmu->regs[2] + offs - 0x228);
>>> + int i;
>>> +
>>> + for (i = 0; i < smmu->nregs; i++) {
>>> + void __iomem *addr = smmu->regbase + offs;
>>> +
>>> + BUG_ON(addr < smmu->regs[i]);
>>> + if (addr <= smmu->rege[i])
>>> + return readl(addr);
>>> + }
>>
>> This loop is purely for checking offset to be valid. And this loop is
>> repeated in the smmu_write() function. I queued a patch on-top to make
>> this more clear. Please double-check:
>
> Actually I did the similar thing in the first version of this patch(*1)
>
> +static inline void smmu_check_reg_range(size_t offs)
> +{
> + int i;
> +
> + for (i = 0; i < smmu->nregs; i++) {
> + BUG_ON(offs < smmu->regs[i] - smmu->regbase);
> + if (offs <= smmu->rege[i] - smmu->regbase)
> + break;
> + }
> +}
>
> *1:
> http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005072.html
>
> Then, Stehpen pointed out about this check function(*2).
>
> "... here, you'd be doing the loop every access anyway, so you may as
> well not calculate regbase at all, move the body of
> smmu_check_reg_range() into smmu_read()/smmu_write(), and do the access
> inside the if statement inside the loop, with the per-range mapping."
Upon reflection, that comment probably isn't correct, since the only way
to know where each register range begins, relative to the register
numbers that the driver uses, is to calculate reg_base. So, I think you
do need reg_base, and Joerg's latest patch seems reasonable.
> *2:
> http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005074.html
>
> I might not get Stehpen's point in the latest patch(?), though....
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <51101BA1.5090208-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
@ 2013-02-04 20:39 ` Hiroshi Doyu
[not found] ` <20130204.223921.2367725583637314.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Hiroshi Doyu @ 2013-02-04 20:39 UTC (permalink / raw)
To: joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org,
swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org
Cc: Stephen Warren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Mon, 4 Feb 2013 21:35:45 +0100:
> >> This loop is purely for checking offset to be valid. And this loop is
> >> repeated in the smmu_write() function. I queued a patch on-top to make
> >> this more clear. Please double-check:
> >
> > Actually I did the similar thing in the first version of this patch(*1)
> >
> > +static inline void smmu_check_reg_range(size_t offs)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < smmu->nregs; i++) {
> > + BUG_ON(offs < smmu->regs[i] - smmu->regbase);
> > + if (offs <= smmu->rege[i] - smmu->regbase)
> > + break;
> > + }
> > +}
> >
> > *1:
> > http://lists.linuxfoundation.org/pipermail/iommu/2013-January/005072.html
> >
> > Then, Stehpen pointed out about this check function(*2).
> >
> > "... here, you'd be doing the loop every access anyway, so you may as
> > well not calculate regbase at all, move the body of
> > smmu_check_reg_range() into smmu_read()/smmu_write(), and do the access
> > inside the if statement inside the loop, with the per-range mapping."
>
> Upon reflection, that comment probably isn't correct, since the only way
> to know where each register range begins, relative to the register
> numbers that the driver uses, is to calculate reg_base. So, I think you
> do need reg_base, and Joerg's latest patch seems reasonable.
Ok, now it's clear. Joerg, please get yours merged on the top of this.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <20130204.223921.2367725583637314.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-04 20:54 ` Hiroshi Doyu
[not found] ` <20130204.225407.2202093543593597795.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 9+ messages in thread
From: Hiroshi Doyu @ 2013-02-04 20:54 UTC (permalink / raw)
To: joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org
Cc: Stephen Warren,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
[-- Attachment #1: Type: text/plain, Size: 542 bytes --]
Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Mon, 04 Feb 2013 22:39:21 +0200 (EET):
> > Upon reflection, that comment probably isn't correct, since the only way
> > to know where each register range begins, relative to the register
> > numbers that the driver uses, is to calculate reg_base. So, I think you
> > do need reg_base, and Joerg's latest patch seems reasonable.
>
> Ok, now it's clear. Joerg, please get yours merged on the top of this.
Joerg, could you please replace v3 with the attached v4?
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-iommu-tegra-smmu-Support-variable-MMIO-ranges-blocks.patch --]
[-- Type: text/x-patch; name="0001-iommu-tegra-smmu-Support-variable-MMIO-ranges-blocks.patch", Size: 4447 bytes --]
From 2f71337453483da291c29b744ca165857c8b84e4 Mon Sep 17 00:00:00 2001
From: Hiroshi Doyu <hdoyu@nvidia.com>
Date: Wed, 9 Jan 2013 12:38:47 +0200
Subject: [v4 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
Presently SMMU registers are located in discontiguous 3 blocks. They
are interleaved by MC registers. Ideally SMMU register blocks should
be in an independent one block, but it is too late to change this H/W
design. In the future Tegra chips over some generations, it is
expected that some of register block "size" can be extended towards
the end and also more new register blocks will be added at most a few
blocks. The starting address of each existing block won't change. This
patch allocates multiple number of register blocks dynamically based
on the info passed from DT. Those ranges are verified in the
accessors{read,write}. This may sacrifice some performance because a
new accessors prevents compiler optimization of a fixed size register
offset calculation. Since SMMU register accesses are not so frequent,
this would be acceptable. This patch is necessary to unify
"tegra-smmu.ko" over some Tegra SoC generations.
Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
Update: Set rege the last valid address.
---
---
drivers/iommu/tegra-smmu.c | 61 ++++++++++++++++++++++++++------------------
1 file changed, 36 insertions(+), 25 deletions(-)
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index fc17889..0d403fe 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -293,7 +293,11 @@ struct smmu_debugfs_info {
* Per SMMU device - IOMMU device
*/
struct smmu_device {
- void __iomem *regs[NUM_SMMU_REG_BANKS];
+ void __iomem *regbase; /* register offset base */
+ void __iomem **regs; /* register block start address array */
+ void __iomem **rege; /* register block end address array */
+ int nregs; /* number of register blocks */
+
unsigned long iovmm_base; /* remappable base address */
unsigned long page_count; /* total remappable size */
spinlock_t lock;
@@ -325,35 +329,33 @@ static struct smmu_device *smmu_handle; /* unique for a system */
*/
static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)
{
- BUG_ON(offs < 0x10);
- if (offs < 0x3c)
- return readl(smmu->regs[0] + offs - 0x10);
- BUG_ON(offs < 0x1f0);
- if (offs < 0x200)
- return readl(smmu->regs[1] + offs - 0x1f0);
- BUG_ON(offs < 0x228);
- if (offs < 0x284)
- return readl(smmu->regs[2] + offs - 0x228);
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(addr < smmu->regs[i]);
+ if (addr <= smmu->rege[i])
+ return readl(addr);
+ }
+
BUG();
}
static inline void smmu_write(struct smmu_device *smmu, u32 val, size_t offs)
{
- BUG_ON(offs < 0x10);
- if (offs < 0x3c) {
- writel(val, smmu->regs[0] + offs - 0x10);
- return;
- }
- BUG_ON(offs < 0x1f0);
- if (offs < 0x200) {
- writel(val, smmu->regs[1] + offs - 0x1f0);
- return;
- }
- BUG_ON(offs < 0x228);
- if (offs < 0x284) {
- writel(val, smmu->regs[2] + offs - 0x228);
- return;
+ int i;
+
+ for (i = 0; i < smmu->nregs; i++) {
+ void __iomem *addr = smmu->regbase + offs;
+
+ BUG_ON(addr < smmu->regs[i]);
+ if (addr <= smmu->rege[i]) {
+ writel(val, addr);
+ return;
+ }
}
+
BUG();
}
@@ -1170,7 +1172,13 @@ static int tegra_smmu_probe(struct platform_device *pdev)
return -ENOMEM;
}
- for (i = 0; i < ARRAY_SIZE(smmu->regs); i++) {
+ smmu->nregs = pdev->num_resources;
+ smmu->regs = devm_kzalloc(dev, 2 * smmu->nregs * sizeof(*smmu->regs),
+ GFP_KERNEL);
+ smmu->rege = smmu->regs + smmu->nregs;
+ if (!smmu->regs)
+ return -ENOMEM;
+ for (i = 0; i < smmu->nregs; i++) {
struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
@@ -1179,7 +1187,10 @@ static int tegra_smmu_probe(struct platform_device *pdev)
smmu->regs[i] = devm_request_and_ioremap(&pdev->dev, res);
if (!smmu->regs[i])
return -EBUSY;
+ smmu->rege[i] = smmu->regs[i] + resource_size(res) - 1;
}
+ /* Same as "mc" 1st regiter block start address */
+ smmu->regbase = (void __iomem *)((u32)smmu->regs[0] & ~PAGE_MASK);
err = of_get_dma_window(dev->of_node, NULL, 0, NULL, &base, &size);
if (err)
--
1.7.9.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks
[not found] ` <20130204.225407.2202093543593597795.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2013-02-05 13:19 ` joro-zLv9SwRftAIdnm+yROfE0A
0 siblings, 0 replies; 9+ messages in thread
From: joro-zLv9SwRftAIdnm+yROfE0A @ 2013-02-05 13:19 UTC (permalink / raw)
To: Hiroshi Doyu
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Stephen Warren
On Mon, Feb 04, 2013 at 09:54:07PM +0100, Hiroshi Doyu wrote:
> Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote @ Mon, 04 Feb 2013 22:39:21 +0200 (EET):
>
> > > Upon reflection, that comment probably isn't correct, since the only way
> > > to know where each register range begins, relative to the register
> > > numbers that the driver uses, is to calculate reg_base. So, I think you
> > > do need reg_base, and Joerg's latest patch seems reasonable.
> >
> > Ok, now it's clear. Joerg, please get yours merged on the top of this.
>
> Joerg, could you please replace v3 with the attached v4?
Done.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-02-05 13:19 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-31 8:14 [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks Hiroshi Doyu
[not found] ` <1359620050-28727-1-git-send-email-hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-04 19:53 ` Joerg Roedel
[not found] ` <20130204195232.GA15278-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2013-02-04 20:31 ` Hiroshi Doyu
[not found] ` <20130204.223114.121259250064618894.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-04 20:35 ` Stephen Warren
[not found] ` <51101BA1.5090208-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-02-04 20:39 ` Hiroshi Doyu
[not found] ` <20130204.223921.2367725583637314.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-04 20:54 ` Hiroshi Doyu
[not found] ` <20130204.225407.2202093543593597795.hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-02-05 13:19 ` joro-zLv9SwRftAIdnm+yROfE0A
-- strict thread matches above, loose matches on Subject: below --
2013-01-29 17:03 [v2 " Stephen Warren
[not found] ` <510800F7.7020507-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2013-01-29 17:56 ` [v3 " Hiroshi Doyu
[not found] ` <1359482169-26756-1-git-send-email-hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2013-01-29 17:58 ` Stephen Warren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).