[v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[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

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Joerg Roedel]

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

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Hiroshi Doyu]

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....

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Stephen Warren]

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....

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Hiroshi Doyu]

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.

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Hiroshi Doyu]

[-- 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

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[joro-zLv9SwRftAIdnm+yROfE0A]

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.

Re: [v2 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Stephen Warren]

On 01/29/2013 01:34 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.

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

>  static inline u32 smmu_read(struct smmu_device *smmu, size_t offs)

> +	for (i = 0; i < smmu->nregs; i++) {
> +		void __iomem *addr = smmu->regbase + offs;

I don't really like the concept of "regbase", but I suppose it works
just fine and isn't entirely avoidable, so I won't object.

> +		BUG_ON(addr < smmu->regs[i]);
> +		if (addr <= smmu->rege[i])
> +			return readl(addr);

So here we assume that rege points at the last valid address in the range...

> @@ -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;

... but here rege is set to the first address after the range. I think
the simplest solution is to change the <= to < in smmu_{read,write}().

> +	/* Same as "mc" 1st regiter block start address */
> +	smmu->regbase = (void __iomem *)((u32)smmu->regs[0] & ~PAGE_MASK);

I'm not sure if it's relevant how these register ranges are related to
the MC registers, given this is the SMMU driver?

s/regiter/register/ in the comment if you keep it.

[v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[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>
---
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

Re: [v3 1/1] iommu/tegra: smmu: Support variable MMIO ranges/blocks

[Stephen Warren]

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>