[PATCHv2 00/16] OMAP IOMMU DT adaptation and cleanup

[PATCHv2 00/16] OMAP IOMMU DT adaptation and cleanup

[Suman Anna]

Hi,

This is an updated series to the initial OMAP IOMMU DT driver
adaptation series [1], that primarily dealt with just the OMAP3
ISP MMU. This series is based on 3.14-rc2, and the patches were
developed in collaboration with Florian. I am hoping that the
series can make the 3.15 merge window.

This series updates the bindings for IOMMUs for all OMAP2+ SoCs
(OMAP3, OMAP4, OMAP5, DRA7), and includes new patches for adding
the support to iommus on OMAP4 and OMAP5, and IVA IOMMU on OMAP3.
The DT bindings and adaptation is done mainly in patches 3 and 4.
The differences between IOMMUs between different OMAP generations
is explained in the previous posting [1], and the differentiation
is achieved through two optional properties keeping the compatible
strings to a minimum. This could also have been achieved through
driver match data with a compatible string per sub-system, so do
let me know if that should be the preferred approach.

All the MMUs other than the OMAP3 ISP leverage omap_device reset
functions, performed through platform data ops previously. With
the removal of the legacy mode, the same functionality is achieved
for DT nodes through pdata quirks until a TI PRCM reset driver is
available.

The first 7 patches in the series are in drivers/iommu, with all the
remaining patches in the arch/arm/mach-omap2 layer.

Tony,
The last 3 patches are cleanup of the legacy mode, so IOMMU devices
cannot be instantiated after these patches. Please let me know if
legacy mode on OMAP3 needs to be supported for 3.15, in which case,
the last 3 can be dropped for now and I would have to revise the OMAP3
ISP archdata change (Patch 9) to support both legacy and DT boots.
At present, I have made the change to support OMAP3 ISP with DT-boot
only.

Detailed changes in v2:
- Cleanup of driver probe/release to use devm_ interfaces (Patch 1)
- The DT bindings are split into a separate patch, and updated based
  on discussion on v1 [1] (Patch 3)
- Updated DT adaptation patch with improved error checking, and
  support for DRA7 compatible IOMMUs in the driver (Patch 4)
- Added support for throwing a bus error response back to the processor
  cores on MMUs associated with IPUs (Patch 5)
- Added preliminary support to DT-based IOMMU users (Patch 6)
- Added preparatory patches to enable and use the hwmod for IVA MMU
  on OMAP3 (Patches 8 and 11)
- Adapt the OMAP3 ISP archdata to support DT boot (Patch 9). Legacy
  mode will not work after this patch, and this will be cleaned up
  anyway once OMAP3ISP is converted to a DT node.
- Reset functionality enablement with DT-boots using pdata quirks
  for OMAP3 IVA, OMAP4 and OMAP5 DSP & IPU MMUs (Patches 10, 13)
- Added the basic hwmod data for OMAP5 iommus (Patch 12)
- Clean up the iommu hwmod data and remove the legacy file for creating
  IOMMU devices (Patches 14, 15, 16).
- Dropped the "iommu/omap: Do bus_set_iommu() only if probe() succeeds"
  patch from previous series.
- Dropped the OMAP3 ISP MMU DTS patch (posting separately).

I have validated the functionality of all the different IOMMUs on
OMAP3, OMAP4 and OMAP5. Florian has verified the OMAP3 ISP usage as
well. The full branch including the DTS patches is here for reference, 
https://github.com/sumananna/omap-kernel/commits/iommu/3.14-rc2-dt-support-v2

v1:
- Couple of cleanup and initial DT adaptation for OMAP3 ISP.
http://marc.info/?l=linux-omap&m=138728485600624&w=2

[1] http://marc.info/?l=linux-omap&m=138782819732435&w=2


Florian Vaussard (8):
  iommu/omap: omap_iommu_attach() should return ENODEV, not NULL
  Documentation: dt: add OMAP iommu bindings
  iommu/omap: add devicetree support
  iommu/omap: allow enable/disable even without pdata
  ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2
  ARM: OMAP3: hwmod data: cleanup data for IOMMUs
  ARM: OMAP4: hwmod data: cleanup data for IOMMUs
  ARM: OMAP2+: Remove legacy omap-iommu.c

Laurent Pinchart (1):
  iommu/omap: allocate archdata on the fly for DT-based devices

Suman Anna (7):
  iommu/omap: convert to devm_* interfaces
  iommu/omap: enable bus-error back on supported iommus
  ARM: OMAP2+: change the ISP device archdata MMU name
  ARM: OMAP2+: use pdata quirks for iommu reset lines
  ARM: OMAP3: fix iva mmu programming issues
  ARM: OMAP5: hwmod data: add mmu data for ipu & dsp
  ARM: OMAP2+: extend iommu pdata-quirks to OMAP5

 .../devicetree/bindings/iommu/ti,omap-iommu.txt    |  28 ++++
 arch/arm/mach-omap2/Makefile                       |   3 -
 arch/arm/mach-omap2/clockdomains3xxx_data.c        |   2 +-
 arch/arm/mach-omap2/devices.c                      |   2 +-
 arch/arm/mach-omap2/omap-iommu.c                   |  74 ----------
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |  58 +-------
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |  35 -----
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c         |  83 +++++++++++
 arch/arm/mach-omap2/pdata-quirks.c                 |  24 ++++
 arch/arm/plat-omap/Kconfig                         |   3 -
 drivers/iommu/omap-iommu.c                         | 154 +++++++++++++--------
 drivers/iommu/omap-iommu.h                         |   5 +
 drivers/iommu/omap-iommu2.c                        |   3 +
 13 files changed, 247 insertions(+), 227 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
 delete mode 100644 arch/arm/mach-omap2/omap-iommu.c

-- 
1.8.5.3

[PATCHv2 01/16] iommu/omap: convert to devm_* interfaces

[Suman Anna]

Use the various devm_ interfaces to simplify the cleanup in
probe and remove functions.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/iommu/omap-iommu.c | 52 +++++++++-------------------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index bcd78a7..fff2ffd 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -941,7 +941,7 @@ static int omap_iommu_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
+	obj = devm_kzalloc(&pdev->dev, sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
 	if (!obj)
 		return -ENOMEM;
 
@@ -958,33 +958,18 @@ static int omap_iommu_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&obj->mmap);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		err = -ENODEV;
-		goto err_mem;
-	}
-
-	res = request_mem_region(res->start, resource_size(res),
-				 dev_name(&pdev->dev));
-	if (!res) {
-		err = -EIO;
-		goto err_mem;
-	}
-
-	obj->regbase = ioremap(res->start, resource_size(res));
-	if (!obj->regbase) {
-		err = -ENOMEM;
-		goto err_ioremap;
-	}
+	obj->regbase = devm_ioremap_resource(obj->dev, res);
+	if (IS_ERR(obj->regbase))
+		return PTR_ERR(obj->regbase);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		err = -ENODEV;
-		goto err_irq;
-	}
-	err = request_irq(irq, iommu_fault_handler, IRQF_SHARED,
-			  dev_name(&pdev->dev), obj);
+	if (irq < 0)
+		return -ENODEV;
+
+	err = devm_request_irq(obj->dev, irq, iommu_fault_handler, IRQF_SHARED,
+			       dev_name(obj->dev), obj);
 	if (err < 0)
-		goto err_irq;
+		return err;
 	platform_set_drvdata(pdev, obj);
 
 	pm_runtime_irq_safe(obj->dev);
@@ -992,34 +977,17 @@ static int omap_iommu_probe(struct platform_device *pdev)
 
 	dev_info(&pdev->dev, "%s registered\n", obj->name);
 	return 0;
-
-err_irq:
-	iounmap(obj->regbase);
-err_ioremap:
-	release_mem_region(res->start, resource_size(res));
-err_mem:
-	kfree(obj);
-	return err;
 }
 
 static int omap_iommu_remove(struct platform_device *pdev)
 {
-	int irq;
-	struct resource *res;
 	struct omap_iommu *obj = platform_get_drvdata(pdev);
 
 	iopgtable_clear_entry_all(obj);
 
-	irq = platform_get_irq(pdev, 0);
-	free_irq(irq, obj);
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	release_mem_region(res->start, resource_size(res));
-	iounmap(obj->regbase);
-
 	pm_runtime_disable(obj->dev);
 
 	dev_info(&pdev->dev, "%s removed\n", obj->name);
-	kfree(obj);
 	return 0;
 }
 
-- 
1.8.5.3

Re: [PATCHv2 01/16] iommu/omap: convert to devm_* interfaces

[Laurent Pinchart]

Hi Suman,

Thank you for the patch.

On Thursday 13 February 2014 12:15:32 Suman Anna wrote:
> Use the various devm_ interfaces to simplify the cleanup in
> probe and remove functions.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
>  drivers/iommu/omap-iommu.c | 52  ++++++++----------------------------------
>  1 file changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index bcd78a7..fff2ffd 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -941,7 +941,7 @@ static int omap_iommu_probe(struct platform_device
> *pdev) struct resource *res;
>  	struct iommu_platform_data *pdata = pdev->dev.platform_data;
> 
> -	obj = kzalloc(sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
> +	obj = devm_kzalloc(&pdev->dev, sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
>  	if (!obj)
>  		return -ENOMEM;
> 
> @@ -958,33 +958,18 @@ static int omap_iommu_probe(struct platform_device
> *pdev) INIT_LIST_HEAD(&obj->mmap);
> 
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		err = -ENODEV;
> -		goto err_mem;
> -	}
> -
> -	res = request_mem_region(res->start, resource_size(res),
> -				 dev_name(&pdev->dev));
> -	if (!res) {
> -		err = -EIO;
> -		goto err_mem;
> -	}
> -
> -	obj->regbase = ioremap(res->start, resource_size(res));
> -	if (!obj->regbase) {
> -		err = -ENOMEM;
> -		goto err_ioremap;
> -	}
> +	obj->regbase = devm_ioremap_resource(obj->dev, res);
> +	if (IS_ERR(obj->regbase))
> +		return PTR_ERR(obj->regbase);
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		err = -ENODEV;
> -		goto err_irq;
> -	}
> -	err = request_irq(irq, iommu_fault_handler, IRQF_SHARED,
> -			  dev_name(&pdev->dev), obj);
> +	if (irq < 0)
> +		return -ENODEV;
> +
> +	err = devm_request_irq(obj->dev, irq, iommu_fault_handler, IRQF_SHARED,
> +			       dev_name(obj->dev), obj);
>  	if (err < 0)
> -		goto err_irq;
> +		return err;
>  	platform_set_drvdata(pdev, obj);
> 
>  	pm_runtime_irq_safe(obj->dev);
> @@ -992,34 +977,17 @@ static int omap_iommu_probe(struct platform_device
> *pdev)
> 
>  	dev_info(&pdev->dev, "%s registered\n", obj->name);
>  	return 0;
> -
> -err_irq:
> -	iounmap(obj->regbase);
> -err_ioremap:
> -	release_mem_region(res->start, resource_size(res));
> -err_mem:
> -	kfree(obj);
> -	return err;
>  }
> 
>  static int omap_iommu_remove(struct platform_device *pdev)
>  {
> -	int irq;
> -	struct resource *res;
>  	struct omap_iommu *obj = platform_get_drvdata(pdev);
> 
>  	iopgtable_clear_entry_all(obj);
> 
> -	irq = platform_get_irq(pdev, 0);
> -	free_irq(irq, obj);
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	release_mem_region(res->start, resource_size(res));
> -	iounmap(obj->regbase);
> -
>  	pm_runtime_disable(obj->dev);
> 
>  	dev_info(&pdev->dev, "%s removed\n", obj->name);
> -	kfree(obj);
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart

[PATCHv2 02/16] iommu/omap: omap_iommu_attach() should return ENODEV, not NULL

[Suman Anna]

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

omap_iommu_attach() returns NULL or ERR_PTR in case of error, but
omap_iommu_attach_dev() only checks for IS_ERR. Thus a NULL return value (in
case driver_find_device fails) will cause the kernel to panic when
omap_iommu_attach_dev() dereferences the pointer.

In such case, omap_iommu_attach() should return ENODEV, not NULL.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
Acked-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 drivers/iommu/omap-iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index fff2ffd..6272c36 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -863,7 +863,7 @@ static int device_match_by_alias(struct device *dev, void *data)
  **/
 static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
 {
-	int err = -ENOMEM;
+	int err = -ENODEV;
 	struct device *dev;
 	struct omap_iommu *obj;
 
@@ -871,7 +871,7 @@ static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
 				(void *)name,
 				device_match_by_alias);
 	if (!dev)
-		return NULL;
+		return ERR_PTR(err);
 
 	obj = to_iommu(dev);
 
-- 
1.8.5.3

Re: [PATCHv2 02/16] iommu/omap: omap_iommu_attach() should return ENODEV, not NULL

[Laurent Pinchart]

Hi Suman,

Thank you for the patch.

On Thursday 13 February 2014 12:15:33 Suman Anna wrote:
> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> 
> omap_iommu_attach() returns NULL or ERR_PTR in case of error, but
> omap_iommu_attach_dev() only checks for IS_ERR. Thus a NULL return value (in
> case driver_find_device fails) will cause the kernel to panic when
> omap_iommu_attach_dev() dereferences the pointer.
> 
> In such case, omap_iommu_attach() should return ENODEV, not NULL.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> Acked-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/iommu/omap-iommu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index fff2ffd..6272c36 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -863,7 +863,7 @@ static int device_match_by_alias(struct device *dev,
> void *data) **/
>  static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
>  {
> -	int err = -ENOMEM;
> +	int err = -ENODEV;
>  	struct device *dev;
>  	struct omap_iommu *obj;
> 
> @@ -871,7 +871,7 @@ static struct omap_iommu *omap_iommu_attach(const char
> *name, u32 *iopgd) (void *)name,
>  				device_match_by_alias);
>  	if (!dev)
> -		return NULL;
> +		return ERR_PTR(err);

I would return ERR_PTR(-ENODEV) here, and remove the initialization at 
declaration of err above.

> 
>  	obj = to_iommu(dev);

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 02/16] iommu/omap: omap_iommu_attach() should return ENODEV, not NULL

[Suman Anna]

Hi Laurent,

On 02/25/2014 03:13 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> Thank you for the patch.
>
> On Thursday 13 February 2014 12:15:33 Suman Anna wrote:
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> omap_iommu_attach() returns NULL or ERR_PTR in case of error, but
>> omap_iommu_attach_dev() only checks for IS_ERR. Thus a NULL return value (in
>> case driver_find_device fails) will cause the kernel to panic when
>> omap_iommu_attach_dev() dereferences the pointer.
>>
>> In such case, omap_iommu_attach() should return ENODEV, not NULL.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>> Acked-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>> ---
>>   drivers/iommu/omap-iommu.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
>> index fff2ffd..6272c36 100644
>> --- a/drivers/iommu/omap-iommu.c
>> +++ b/drivers/iommu/omap-iommu.c
>> @@ -863,7 +863,7 @@ static int device_match_by_alias(struct device *dev,
>> void *data) **/
>>   static struct omap_iommu *omap_iommu_attach(const char *name, u32 *iopgd)
>>   {
>> -	int err = -ENOMEM;
>> +	int err = -ENODEV;
>>   	struct device *dev;
>>   	struct omap_iommu *obj;
>>
>> @@ -871,7 +871,7 @@ static struct omap_iommu *omap_iommu_attach(const char
>> *name, u32 *iopgd) (void *)name,
>>   				device_match_by_alias);
>>   	if (!dev)
>> -		return NULL;
>> +		return ERR_PTR(err);
>
> I would return ERR_PTR(-ENODEV) here, and remove the initialization at
> declaration of err above.

The initialization at the beginning is also serving one another exit 
path (a check for try_module_get), where err is not being set. If the 
initialization is removed, then the err has to be set explicitly at the 
other place. Let me know if you still want this changed.

regards
Suman

>
>>
>>   	obj = to_iommu(dev);
>

Re: [PATCHv2 02/16] iommu/omap: omap_iommu_attach() should return ENODEV, not NULL

[Laurent Pinchart]

Hi Suman,

On Tuesday 25 February 2014 16:32:03 Suman Anna wrote:
> On 02/25/2014 03:13 PM, Laurent Pinchart wrote:
> > On Thursday 13 February 2014 12:15:33 Suman Anna wrote:
> >> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >> 
> >> omap_iommu_attach() returns NULL or ERR_PTR in case of error, but
> >> omap_iommu_attach_dev() only checks for IS_ERR. Thus a NULL return value
> >> (in case driver_find_device fails) will cause the kernel to panic when
> >> omap_iommu_attach_dev() dereferences the pointer.
> >> 
> >> In such case, omap_iommu_attach() should return ENODEV, not NULL.
> >> 
> >> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >> Acked-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> >> ---
> >> 
> >>   drivers/iommu/omap-iommu.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> >> index fff2ffd..6272c36 100644
> >> --- a/drivers/iommu/omap-iommu.c
> >> +++ b/drivers/iommu/omap-iommu.c
> >> @@ -863,7 +863,7 @@ static int device_match_by_alias(struct device *dev,
> >> void *data) **/
> >> 
> >>   static struct omap_iommu *omap_iommu_attach(const char *name, u32
> >>   *iopgd)
> >>   {
> >> -	int err = -ENOMEM;
> >> +	int err = -ENODEV;
> >>   	struct device *dev;
> >>   	struct omap_iommu *obj;
> >> 
> >> @@ -871,7 +871,7 @@ static struct omap_iommu *omap_iommu_attach(const
> >> char *name, u32 *iopgd)
> >>   				(void *)name,
> >>   				device_match_by_alias);
> >>   	if (!dev)
> >> -		return NULL;
> >> +		return ERR_PTR(err);
> > 
> > I would return ERR_PTR(-ENODEV) here, and remove the initialization at
> > declaration of err above.
> 
> The initialization at the beginning is also serving one another exit
> path (a check for try_module_get), where err is not being set. If the
> initialization is removed, then the err has to be set explicitly at the
> other place. Let me know if you still want this changed.

The return value of iommu_enable() is unconditionally stored in err before the 
try_module_get() call, so that code patch is buggy anyway and should be fixed. 
I would still remove the initialization at declaration and assign -ENODEV to 
err explicitly when try_module_get() fails before the goto err_module.

> >>   	obj = to_iommu(dev);

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 02/16] iommu/omap: omap_iommu_attach() should return ENODEV, not NULL

[Suman Anna]

Hi Laurent,

> On Tuesday 25 February 2014 16:32:03 Suman Anna wrote:
>> On 02/25/2014 03:13 PM, Laurent Pinchart wrote:
>>> On Thursday 13 February 2014 12:15:33 Suman Anna wrote:
>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>
>>>> omap_iommu_attach() returns NULL or ERR_PTR in case of error, but
>>>> omap_iommu_attach_dev() only checks for IS_ERR. Thus a NULL return value
>>>> (in case driver_find_device fails) will cause the kernel to panic when
>>>> omap_iommu_attach_dev() dereferences the pointer.
>>>>
>>>> In such case, omap_iommu_attach() should return ENODEV, not NULL.
>>>>
>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>> Acked-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>> ---
>>>>
>>>>    drivers/iommu/omap-iommu.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
>>>> index fff2ffd..6272c36 100644
>>>> --- a/drivers/iommu/omap-iommu.c
>>>> +++ b/drivers/iommu/omap-iommu.c
>>>> @@ -863,7 +863,7 @@ static int device_match_by_alias(struct device *dev,
>>>> void *data) **/
>>>>
>>>>    static struct omap_iommu *omap_iommu_attach(const char *name, u32
>>>>    *iopgd)
>>>>    {
>>>> -	int err = -ENOMEM;
>>>> +	int err = -ENODEV;
>>>>    	struct device *dev;
>>>>    	struct omap_iommu *obj;
>>>>
>>>> @@ -871,7 +871,7 @@ static struct omap_iommu *omap_iommu_attach(const
>>>> char *name, u32 *iopgd)
>>>>    				(void *)name,
>>>>    				device_match_by_alias);
>>>>    	if (!dev)
>>>> -		return NULL;
>>>> +		return ERR_PTR(err);
>>>
>>> I would return ERR_PTR(-ENODEV) here, and remove the initialization at
>>> declaration of err above.
>>
>> The initialization at the beginning is also serving one another exit
>> path (a check for try_module_get), where err is not being set. If the
>> initialization is removed, then the err has to be set explicitly at the
>> other place. Let me know if you still want this changed.
>
> The return value of iommu_enable() is unconditionally stored in err before the
> try_module_get() call, so that code patch is buggy anyway and should be fixed.
> I would still remove the initialization at declaration and assign -ENODEV to
> err explicitly when try_module_get() fails before the goto err_module.

OK, I will fix this up.

regards
Suman

[PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
the standard bindings used by OMAP peripherals, this patch uses a
'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
[s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt

diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
new file mode 100644
index 0000000..116492d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
@@ -0,0 +1,28 @@
+OMAP2+ IOMMU
+
+Required properties:
+- compatible : Should be one of,
+		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
+		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
+		"ti,dra7-iommu" for DRA7xx IOMMU instances
+- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
+- reg        : Address space for the configuration registers
+- interrupts : Interrupt specifier for the IOMMU instance
+- dma-window : IOVA start address and length
+
+Optional properties:
+- ti,#tlb-entries : Number of entries in the translation look-aside buffer.
+                    Should be either 8 or 32 (default: 32)
+- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
+		          back a bus error response on MMU faults.
+
+Example:
+	/* OMAP3 ISP MMU */
+	mmu_isp: mmu@480bd400 {
+		compatible = "ti,omap2-iommu";
+		reg = <0x480bd400 0x80>;
+		interrupts = <24>;
+		ti,hwmods = "mmu_isp";
+		ti,#tlb-entries = <8>;
+		dma-window = <0 0xfffff000>;
+	};
-- 
1.8.5.3

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Florian Vaussard]

Hi,

On 02/13/2014 07:15 PM, Suman Anna wrote:
> From: Florian Vaussard <florian.vaussard@epfl.ch>
> 
> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> the standard bindings used by OMAP peripherals, this patch uses a
> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> [s-anna@ti.com: split bindings document, add dra7 and bus error back]
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> 
> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> new file mode 100644
> index 0000000..116492d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> @@ -0,0 +1,28 @@
> +OMAP2+ IOMMU
> +
> +Required properties:
> +- compatible : Should be one of,
> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> +- reg        : Address space for the configuration registers
> +- interrupts : Interrupt specifier for the IOMMU instance
> +- dma-window : IOVA start address and length
> +
> +Optional properties:
> +- ti,#tlb-entries : Number of entries in the translation look-aside buffer.
> +                    Should be either 8 or 32 (default: 32)
> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
> +		          back a bus error response on MMU faults.
> +
> +Example:
> +	/* OMAP3 ISP MMU */
> +	mmu_isp: mmu@480bd400 {
> +		compatible = "ti,omap2-iommu";
> +		reg = <0x480bd400 0x80>;
> +		interrupts = <24>;
> +		ti,hwmods = "mmu_isp";
> +		ti,#tlb-entries = <8>;
> +		dma-window = <0 0xfffff000>;
> +	};
> 

Any comments on this binding?

Regards,
Florian

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

Mark, Kumar, Rob,

> On 02/13/2014 07:15 PM, Suman Anna wrote:
>> From: Florian Vaussard <florian.vaussard@epfl.ch>
>>
>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>> the standard bindings used by OMAP peripherals, this patch uses a
>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> [s-anna@ti.com: split bindings document, add dra7 and bus error back]
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>> new file mode 100644
>> index 0000000..116492d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>> @@ -0,0 +1,28 @@
>> +OMAP2+ IOMMU
>> +
>> +Required properties:
>> +- compatible : Should be one of,
>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
>> +- reg        : Address space for the configuration registers
>> +- interrupts : Interrupt specifier for the IOMMU instance
>> +- dma-window : IOVA start address and length
>> +
>> +Optional properties:
>> +- ti,#tlb-entries : Number of entries in the translation look-aside buffer.
>> +                    Should be either 8 or 32 (default: 32)
>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
>> +		          back a bus error response on MMU faults.
>> +
>> +Example:
>> +	/* OMAP3 ISP MMU */
>> +	mmu_isp: mmu@480bd400 {
>> +		compatible = "ti,omap2-iommu";
>> +		reg = <0x480bd400 0x80>;
>> +		interrupts = <24>;
>> +		ti,hwmods = "mmu_isp";
>> +		ti,#tlb-entries = <8>;
>> +		dma-window = <0 0xfffff000>;
>> +	};
>>
>
> Any comments on this binding?

Can one of you look through this binding and ack/suggest changes?
As I mentioned in the cover letter, the optional properties usually 
apply only to specific subsystems, so do let me know if this should be 
handled using compatible string per subsystem per SoC rather than per SoC.

regards
Suman

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Laurent Pinchart]

Hi Suman,

Thank you for the patch.

On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> 
> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> the standard bindings used by OMAP peripherals, this patch uses a
> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> ---
>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644
>  Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> 
> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode
> 100644
> index 0000000..116492d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> @@ -0,0 +1,28 @@
> +OMAP2+ IOMMU
> +
> +Required properties:
> +- compatible : Should be one of,
> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> +- reg        : Address space for the configuration registers
> +- interrupts : Interrupt specifier for the IOMMU instance
> +- dma-window : IOVA start address and length

Isn't the dma window more of a system configuration property than a hardware 
property ? How do you expect it to be set ?

> +Optional properties:
> +- ti,#tlb-entries : Number of entries in the translation look-aside buffer.
> +                    Should be either 8 or 32 (default: 32)
> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
> +		          back a bus error response on MMU faults.

Do these features vary per IOMMU instance or per IOMMU model ? In the latter 
case they could be inferred from the compatible string by the driver without 
requiring them to be explicit in DT (whether you want to do so is left to you 
though).

> +Example:
> +	/* OMAP3 ISP MMU */
> +	mmu_isp: mmu@480bd400 {
> +		compatible = "ti,omap2-iommu";
> +		reg = <0x480bd400 0x80>;
> +		interrupts = <24>;
> +		ti,hwmods = "mmu_isp";
> +		ti,#tlb-entries = <8>;
> +		dma-window = <0 0xfffff000>;
> +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

Hi Laurent,

On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> Thank you for the patch.
>
> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>> the standard bindings used by OMAP peripherals, this patch uses a
>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>> ---
>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 +++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>   create mode 100644
>>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode
>> 100644
>> index 0000000..116492d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>> @@ -0,0 +1,28 @@
>> +OMAP2+ IOMMU
>> +
>> +Required properties:
>> +- compatible : Should be one of,
>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
>> +- reg        : Address space for the configuration registers
>> +- interrupts : Interrupt specifier for the IOMMU instance
>> +- dma-window : IOVA start address and length
>
> Isn't the dma window more of a system configuration property than a hardware
> property ? How do you expect it to be set?

We are setting it based on the addressable range for the MMU. We are 
reusing the existing defined property and it allows us to get rid of the 
IOVA start and end addresses defined in the pre-DT OMAP iommu platform data.

>
>> +Optional properties:
>> +- ti,#tlb-entries : Number of entries in the translation look-aside buffer.
>> +                    Should be either 8 or 32 (default: 32)
>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
>> +		          back a bus error response on MMU faults.
>
> Do these features vary per IOMMU instance or per IOMMU model ? In the latter
> case they could be inferred from the compatible string by the driver without
> requiring them to be explicit in DT (whether you want to do so is left to you
> though).

Well, these are fixed features given an IOMMU instance, like the OMAP3 
ISP is the only one that has 8 TLB entries, all the remaining ones have 
32, and the IPU iommu instances are the only ones that support the bus 
error response back. I have no preference to any particular way, and 
sure the driver can infer these easily based on unique compatible 
strings per subsystem per SoC. I just happened to go with defining 
compatible strings per SoC, with the optional properties differentiating 
the fixed behavior between different IOMMU instances on that SoC. This 
is where I was looking for some inputs/guidance from the DT bindings 
maintainers on what is the preferred method.

regards
Suman

>
>> +Example:
>> +	/* OMAP3 ISP MMU */
>> +	mmu_isp: mmu@480bd400 {
>> +		compatible = "ti,omap2-iommu";
>> +		reg = <0x480bd400 0x80>;
>> +		interrupts = <24>;
>> +		ti,hwmods = "mmu_isp";
>> +		ti,#tlb-entries = <8>;
>> +		dma-window = <0 0xfffff000>;
>> +	};
>

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Laurent Pinchart]

Hi Suman,

On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> > On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >> From: Florian Vaussard <florian.vaussard@epfl.ch>
> >> 
> >> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >> the standard bindings used by OMAP peripherals, this patch uses a
> >> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >> 
> >> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> >> [s-anna@ti.com: split bindings document, add dra7 and bus error back]
> >> Signed-off-by: Suman Anna <s-anna@ti.com>
> >> ---
> >> 
> >>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28
> >>   +++++++++++++++++++
> >>   1 file changed, 28 insertions(+)
> >>   create mode 100644
> >>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode
> >> 100644
> >> index 0000000..116492d
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >> @@ -0,0 +1,28 @@
> >> +OMAP2+ IOMMU
> >> +
> >> +Required properties:
> >> +- compatible : Should be one of,
> >> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> >> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> >> +- reg        : Address space for the configuration registers
> >> +- interrupts : Interrupt specifier for the IOMMU instance
> >> +- dma-window : IOVA start address and length
> > 
> > Isn't the dma window more of a system configuration property than a
> > hardware property ? How do you expect it to be set?
> 
> We are setting it based on the addressable range for the MMU.

A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both support 
the full 4GB VA space. Why do you need to restrict it ?

> We are reusing the existing defined property and it allows us to get rid of
> the IOVA start and end addresses defined in the pre-DT OMAP iommu platform
> data.
>
> >> +Optional properties:
> >> +- ti,#tlb-entries : Number of entries in the translation look-aside
> >> buffer. +                    Should be either 8 or 32 (default: 32)
> >> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
> >> +		          back a bus error response on MMU faults.
> > 
> > Do these features vary per IOMMU instance or per IOMMU model ? In the
> > latter case they could be inferred from the compatible string by the
> > driver without requiring them to be explicit in DT (whether you want to
> > do so is left to you though).
> 
> Well, these are fixed features given an IOMMU instance, like the OMAP3
> ISP is the only one that has 8 TLB entries, all the remaining ones have
> 32, and the IPU iommu instances are the only ones that support the bus
> error response back. I have no preference to any particular way, and
> sure the driver can infer these easily based on unique compatible
> strings per subsystem per SoC. I just happened to go with defining
> compatible strings per SoC, with the optional properties differentiating
> the fixed behavior between different IOMMU instances on that SoC. This
> is where I was looking for some inputs/guidance from the DT bindings
> maintainers on what is the preferred method.

I think you've made the right choice. I wasn't sure whether those parameters 
varied across IOMMU instances of compatible devices (from a compatible string 
point of view) or were constant. As they vary they should be expressed in DT.

> >> +Example:
> >> +	/* OMAP3 ISP MMU */
> >> +	mmu_isp: mmu@480bd400 {
> >> +		compatible = "ti,omap2-iommu";
> >> +		reg = <0x480bd400 0x80>;
> >> +		interrupts = <24>;
> >> +		ti,hwmods = "mmu_isp";
> >> +		ti,#tlb-entries = <8>;
> >> +		dma-window = <0 0xfffff000>;
> >> +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

Hi Laurent,

On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>
>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>
>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>> ---
>>>>
>>>>    .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28
>>>>    +++++++++++++++++++
>>>>    1 file changed, 28 insertions(+)
>>>>    create mode 100644
>>>>    Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file mode
>>>> 100644
>>>> index 0000000..116492d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>> @@ -0,0 +1,28 @@
>>>> +OMAP2+ IOMMU
>>>> +
>>>> +Required properties:
>>>> +- compatible : Should be one of,
>>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
>>>> +- reg        : Address space for the configuration registers
>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>> +- dma-window : IOVA start address and length
>>>
>>> Isn't the dma window more of a system configuration property than a
>>> hardware property ? How do you expect it to be set?
>>
>> We are setting it based on the addressable range for the MMU.
>
> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both support
> the full 4GB VA space. Why do you need to restrict it ?

I should have rephrased it better when I said addressable range. While 
the MMUs are capable of programming the full 4GB space, there are some 
address ranges that are private from the processor view. This window is 
currently used to set the range for the omap-iovmm driver (which only 
OMAP3 ISP is using atm), and there is no point in allowing the 
omap-iovmm driver the full range when the processor could never 
reach/access those addresses.

>
>> We are reusing the existing defined property and it allows us to get rid of
>> the IOVA start and end addresses defined in the pre-DT OMAP iommu platform
>> data.
>>
>>>> +Optional properties:
>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
>>>> buffer. +                    Should be either 8 or 32 (default: 32)
>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports throwing
>>>> +		          back a bus error response on MMU faults.
>>>
>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
>>> latter case they could be inferred from the compatible string by the
>>> driver without requiring them to be explicit in DT (whether you want to
>>> do so is left to you though).
>>
>> Well, these are fixed features given an IOMMU instance, like the OMAP3
>> ISP is the only one that has 8 TLB entries, all the remaining ones have
>> 32, and the IPU iommu instances are the only ones that support the bus
>> error response back. I have no preference to any particular way, and
>> sure the driver can infer these easily based on unique compatible
>> strings per subsystem per SoC. I just happened to go with defining
>> compatible strings per SoC, with the optional properties differentiating
>> the fixed behavior between different IOMMU instances on that SoC. This
>> is where I was looking for some inputs/guidance from the DT bindings
>> maintainers on what is the preferred method.
>
> I think you've made the right choice. I wasn't sure whether those parameters
> varied across IOMMU instances of compatible devices (from a compatible string
> point of view) or were constant. As they vary they should be expressed in DT.

Yeah, I wasn't sure if these qualify as features (as per 
Documentation/devicetree/bindings/ABI.txt section II.2).

regards
Suman

>
>>>> +Example:
>>>> +	/* OMAP3 ISP MMU */
>>>> +	mmu_isp: mmu@480bd400 {
>>>> +		compatible = "ti,omap2-iommu";
>>>> +		reg = <0x480bd400 0x80>;
>>>> +		interrupts = <24>;
>>>> +		ti,hwmods = "mmu_isp";
>>>> +		ti,#tlb-entries = <8>;
>>>> +		dma-window = <0 0xfffff000>;
>>>> +	};
>

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Laurent Pinchart]

Hi Suman,

On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> > On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> >> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> >>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >>>> 
> >>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >>>> the standard bindings used by OMAP peripherals, this patch uses a
> >>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >>>> 
> >>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
> >>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> >>>> ---
> >>>> 
> >>>>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++
> >>>>  1 file changed, 28 insertions(+)
> >>>>  create mode 100644
> >>>>  Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>> 
> >>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
> >>>> mode 100644
> >>>> index 0000000..116492d
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>> @@ -0,0 +1,28 @@
> >>>> +OMAP2+ IOMMU
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible : Should be one of,
> >>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> >>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> >>>> +- reg        : Address space for the configuration registers
> >>>> +- interrupts : Interrupt specifier for the IOMMU instance
> >>>> +- dma-window : IOVA start address and length
> >>> 
> >>> Isn't the dma window more of a system configuration property than a
> >>> hardware property ? How do you expect it to be set?
> >> 
> >> We are setting it based on the addressable range for the MMU.
> > 
> > A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
> > support the full 4GB VA space. Why do you need to restrict it ?
> 
> I should have rephrased it better when I said addressable range. While
> the MMUs are capable of programming the full 4GB space, there are some
> address ranges that are private from the processor view. This window is
> currently used to set the range for the omap-iovmm driver (which only
> OMAP3 ISP is using atm), and there is no point in allowing the
> omap-iovmm driver the full range when the processor could never
> reach/access those addresses.

But the IOMMU VA space is from a device point of view, not from a CPU point of 
view. Could you point me to where those private ranges are documented, in 
order to understand the problem correctly ?

> >> We are reusing the existing defined property and it allows us to get rid
> >> of the IOVA start and end addresses defined in the pre-DT OMAP iommu
> >> platform data.
> >> 
> >>>> +Optional properties:
> >>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
> >>>> buffer. +                    Should be either 8 or 32 (default: 32)
> >>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
> >>>> throwing
> >>>> +		          back a bus error response on MMU faults.
> >>> 
> >>> Do these features vary per IOMMU instance or per IOMMU model ? In the
> >>> latter case they could be inferred from the compatible string by the
> >>> driver without requiring them to be explicit in DT (whether you want to
> >>> do so is left to you though).
> >> 
> >> Well, these are fixed features given an IOMMU instance, like the OMAP3
> >> ISP is the only one that has 8 TLB entries, all the remaining ones have
> >> 32, and the IPU iommu instances are the only ones that support the bus
> >> error response back. I have no preference to any particular way, and
> >> sure the driver can infer these easily based on unique compatible
> >> strings per subsystem per SoC. I just happened to go with defining
> >> compatible strings per SoC, with the optional properties differentiating
> >> the fixed behavior between different IOMMU instances on that SoC. This
> >> is where I was looking for some inputs/guidance from the DT bindings
> >> maintainers on what is the preferred method.
> > 
> > I think you've made the right choice. I wasn't sure whether those
> > parameters varied across IOMMU instances of compatible devices (from a
> > compatible string point of view) or were constant. As they vary they
> > should be expressed in DT.
>
> Yeah, I wasn't sure if these qualify as features (as per
> Documentation/devicetree/bindings/ABI.txt section II.2).
> 
> regards
> Suman
> 
> >>>> +Example:
> >>>> +	/* OMAP3 ISP MMU */
> >>>> +	mmu_isp: mmu@480bd400 {
> >>>> +		compatible = "ti,omap2-iommu";
> >>>> +		reg = <0x480bd400 0x80>;
> >>>> +		interrupts = <24>;
> >>>> +		ti,hwmods = "mmu_isp";
> >>>> +		ti,#tlb-entries = <8>;
> >>>> +		dma-window = <0 0xfffff000>;
> >>>> +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

Hi Laurent,

>
> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>
>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>
>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>> ---
>>>>>>
>>>>>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++
>>>>>>   1 file changed, 28 insertions(+)
>>>>>>   create mode 100644
>>>>>>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
>>>>>> mode 100644
>>>>>> index 0000000..116492d
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>> @@ -0,0 +1,28 @@
>>>>>> +OMAP2+ IOMMU
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible : Should be one of,
>>>>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
>>>>>> +- reg        : Address space for the configuration registers
>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>> +- dma-window : IOVA start address and length
>>>>>
>>>>> Isn't the dma window more of a system configuration property than a
>>>>> hardware property ? How do you expect it to be set?
>>>>
>>>> We are setting it based on the addressable range for the MMU.
>>>
>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>> support the full 4GB VA space. Why do you need to restrict it ?
>>
>> I should have rephrased it better when I said addressable range. While
>> the MMUs are capable of programming the full 4GB space, there are some
>> address ranges that are private from the processor view. This window is
>> currently used to set the range for the omap-iovmm driver (which only
>> OMAP3 ISP is using atm), and there is no point in allowing the
>> omap-iovmm driver the full range when the processor could never
>> reach/access those addresses.
>
> But the IOMMU VA space is from a device point of view, not from a CPU point of
> view. Could you point me to where those private ranges are documented, in
> order to understand the problem correctly ?

Yes, they are indeed from the device perspective. I meant DSP and/or IPU 
by processor.

For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP 
Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external 
addressable range starts from 0x11000000.

regards
Suman

>
>>>> We are reusing the existing defined property and it allows us to get rid
>>>> of the IOVA start and end addresses defined in the pre-DT OMAP iommu
>>>> platform data.
>>>>
>>>>>> +Optional properties:
>>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
>>>>>> buffer. +                    Should be either 8 or 32 (default: 32)
>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
>>>>>> throwing
>>>>>> +		          back a bus error response on MMU faults.
>>>>>
>>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
>>>>> latter case they could be inferred from the compatible string by the
>>>>> driver without requiring them to be explicit in DT (whether you want to
>>>>> do so is left to you though).
>>>>
>>>> Well, these are fixed features given an IOMMU instance, like the OMAP3
>>>> ISP is the only one that has 8 TLB entries, all the remaining ones have
>>>> 32, and the IPU iommu instances are the only ones that support the bus
>>>> error response back. I have no preference to any particular way, and
>>>> sure the driver can infer these easily based on unique compatible
>>>> strings per subsystem per SoC. I just happened to go with defining
>>>> compatible strings per SoC, with the optional properties differentiating
>>>> the fixed behavior between different IOMMU instances on that SoC. This
>>>> is where I was looking for some inputs/guidance from the DT bindings
>>>> maintainers on what is the preferred method.
>>>
>>> I think you've made the right choice. I wasn't sure whether those
>>> parameters varied across IOMMU instances of compatible devices (from a
>>> compatible string point of view) or were constant. As they vary they
>>> should be expressed in DT.
>>
>> Yeah, I wasn't sure if these qualify as features (as per
>> Documentation/devicetree/bindings/ABI.txt section II.2).
>>
>> regards
>> Suman
>>
>>>>>> +Example:
>>>>>> +	/* OMAP3 ISP MMU */
>>>>>> +	mmu_isp: mmu@480bd400 {
>>>>>> +		compatible = "ti,omap2-iommu";
>>>>>> +		reg = <0x480bd400 0x80>;
>>>>>> +		interrupts = <24>;
>>>>>> +		ti,hwmods = "mmu_isp";
>>>>>> +		ti,#tlb-entries = <8>;
>>>>>> +		dma-window = <0 0xfffff000>;
>>>>>> +	};
>

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Laurent Pinchart]

Hi Suman,

On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
> > On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
> >> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> >>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> >>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> >>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>> 
> >>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >>>>>> the standard bindings used by OMAP peripherals, this patch uses a
> >>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >>>>>> 
> >>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back]
> >>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
> >>>>>> ---
> >>>>>> 
> >>>>>>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++
> >>>>>>  1 file changed, 28 insertions(+) 
> >>>>>>  create mode 100644
> >>>>>>  Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>> 
> >>>>>> diff --git
> >>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
> >>>>>> mode 100644
> >>>>>> index 0000000..116492d
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>> @@ -0,0 +1,28 @@
> >>>>>> +OMAP2+ IOMMU
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +- compatible : Should be one of,
> >>>>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >>>>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >>>>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> >>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> >>>>>> +- reg        : Address space for the configuration registers
> >>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
> >>>>>> +- dma-window : IOVA start address and length
> >>>>> 
> >>>>> Isn't the dma window more of a system configuration property than a
> >>>>> hardware property ? How do you expect it to be set?
> >>>> 
> >>>> We are setting it based on the addressable range for the MMU.
> >>> 
> >>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
> >>> support the full 4GB VA space. Why do you need to restrict it ?
> >> 
> >> I should have rephrased it better when I said addressable range. While
> >> the MMUs are capable of programming the full 4GB space, there are some
> >> address ranges that are private from the processor view. This window is
> >> currently used to set the range for the omap-iovmm driver (which only
> >> OMAP3 ISP is using atm), and there is no point in allowing the
> >> omap-iovmm driver the full range when the processor could never
> >> reach/access those addresses.
> > 
> > But the IOMMU VA space is from a device point of view, not from a CPU
> > point of view. Could you point me to where those private ranges are
> > documented, in order to understand the problem correctly ?
> 
> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
> by processor.
> 
> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
> addressable range starts from 0x11000000.

OK, so it looks more like a property of the IOMMU master than a property of 
the IOMMU itself. It would be better to express it as such, but I wonder how 
that could be done, and if it would be worth it in this case.

As not all masters (the OMAP3 ISP doesn't for instance) have restrictions 
regarding the VA range they can address, should this property be at least made 
optional ?

> >>>> We are reusing the existing defined property and it allows us to get
> >>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
> >>>> iommu platform data.
> >>>> 
> >>>>>> +Optional properties:
> >>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
> >>>>>> buffer. +                    Should be either 8 or 32 (default: 32)
> >>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
> >>>>>> throwing
> >>>>>> +		          back a bus error response on MMU faults.
> >>>>> 
> >>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
> >>>>> latter case they could be inferred from the compatible string by the
> >>>>> driver without requiring them to be explicit in DT (whether you want
> >>>>> to do so is left to you though).
> >>>> 
> >>>> Well, these are fixed features given an IOMMU instance, like the OMAP3
> >>>> ISP is the only one that has 8 TLB entries, all the remaining ones have
> >>>> 32, and the IPU iommu instances are the only ones that support the bus
> >>>> error response back. I have no preference to any particular way, and
> >>>> sure the driver can infer these easily based on unique compatible
> >>>> strings per subsystem per SoC. I just happened to go with defining
> >>>> compatible strings per SoC, with the optional properties
> >>>> differentiating the fixed behavior between different IOMMU instances on
> >>>> that SoC. This is where I was looking for some inputs/guidance from the
> >>>> DT bindings maintainers on what is the preferred method.
> >>> 
> >>> I think you've made the right choice. I wasn't sure whether those
> >>> parameters varied across IOMMU instances of compatible devices (from a
> >>> compatible string point of view) or were constant. As they vary they
> >>> should be expressed in DT.
> >> 
> >> Yeah, I wasn't sure if these qualify as features (as per
> >> Documentation/devicetree/bindings/ABI.txt section II.2).
> >> 
> >> regards
> >> Suman
> >> 
> >>>>>> +Example:
> >>>>>> +	/* OMAP3 ISP MMU */
> >>>>>> +	mmu_isp: mmu@480bd400 {
> >>>>>> +		compatible = "ti,omap2-iommu";
> >>>>>> +		reg = <0x480bd400 0x80>;
> >>>>>> +		interrupts = <24>;
> >>>>>> +		ti,hwmods = "mmu_isp";
> >>>>>> +		ti,#tlb-entries = <8>;
> >>>>>> +		dma-window = <0 0xfffff000>;
> >>>>>> +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

Hi Laurent,

On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>
>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>>>
>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
>>>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++
>>>>>>>>   1 file changed, 28 insertions(+)
>>>>>>>>   create mode 100644
>>>>>>>>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>
>>>>>>>> diff --git
>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
>>>>>>>> mode 100644
>>>>>>>> index 0000000..116492d
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>> +OMAP2+ IOMMU
>>>>>>>> +
>>>>>>>> +Required properties:
>>>>>>>> +- compatible : Should be one of,
>>>>>>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
>>>>>>>> +- reg        : Address space for the configuration registers
>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>>>> +- dma-window : IOVA start address and length
>>>>>>>
>>>>>>> Isn't the dma window more of a system configuration property than a
>>>>>>> hardware property ? How do you expect it to be set?
>>>>>>
>>>>>> We are setting it based on the addressable range for the MMU.
>>>>>
>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>>>> support the full 4GB VA space. Why do you need to restrict it ?
>>>>
>>>> I should have rephrased it better when I said addressable range. While
>>>> the MMUs are capable of programming the full 4GB space, there are some
>>>> address ranges that are private from the processor view. This window is
>>>> currently used to set the range for the omap-iovmm driver (which only
>>>> OMAP3 ISP is using atm), and there is no point in allowing the
>>>> omap-iovmm driver the full range when the processor could never
>>>> reach/access those addresses.
>>>
>>> But the IOMMU VA space is from a device point of view, not from a CPU
>>> point of view. Could you point me to where those private ranges are
>>> documented, in order to understand the problem correctly ?
>>
>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
>> by processor.
>>
>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
>> addressable range starts from 0x11000000.
>
> OK, so it looks more like a property of the IOMMU master than a property of
> the IOMMU itself. It would be better to express it as such, but I wonder how
> that could be done, and if it would be worth it in this case.

This property is currently solely used to configure the range for the 
omap-iovmm module, which were supplied through platform data in the 
non-DT case. I am wondering if the way to go here is to use 
iommu_domain_set_attr() and use the domain geometry values.

regards
Suman

>
> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
> regarding the VA range they can address, should this property be at least made
> optional ?
>
>>>>>> We are reusing the existing defined property and it allows us to get
>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
>>>>>> iommu platform data.
>>>>>>
>>>>>>>> +Optional properties:
>>>>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
>>>>>>>> buffer. +                    Should be either 8 or 32 (default: 32)
>>>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
>>>>>>>> throwing
>>>>>>>> +		          back a bus error response on MMU faults.
>>>>>>>
>>>>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
>>>>>>> latter case they could be inferred from the compatible string by the
>>>>>>> driver without requiring them to be explicit in DT (whether you want
>>>>>>> to do so is left to you though).
>>>>>>
>>>>>> Well, these are fixed features given an IOMMU instance, like the OMAP3
>>>>>> ISP is the only one that has 8 TLB entries, all the remaining ones have
>>>>>> 32, and the IPU iommu instances are the only ones that support the bus
>>>>>> error response back. I have no preference to any particular way, and
>>>>>> sure the driver can infer these easily based on unique compatible
>>>>>> strings per subsystem per SoC. I just happened to go with defining
>>>>>> compatible strings per SoC, with the optional properties
>>>>>> differentiating the fixed behavior between different IOMMU instances on
>>>>>> that SoC. This is where I was looking for some inputs/guidance from the
>>>>>> DT bindings maintainers on what is the preferred method.
>>>>>
>>>>> I think you've made the right choice. I wasn't sure whether those
>>>>> parameters varied across IOMMU instances of compatible devices (from a
>>>>> compatible string point of view) or were constant. As they vary they
>>>>> should be expressed in DT.
>>>>
>>>> Yeah, I wasn't sure if these qualify as features (as per
>>>> Documentation/devicetree/bindings/ABI.txt section II.2).
>>>>
>>>> regards
>>>> Suman
>>>>
>>>>>>>> +Example:
>>>>>>>> +	/* OMAP3 ISP MMU */
>>>>>>>> +	mmu_isp: mmu@480bd400 {
>>>>>>>> +		compatible = "ti,omap2-iommu";
>>>>>>>> +		reg = <0x480bd400 0x80>;
>>>>>>>> +		interrupts = <24>;
>>>>>>>> +		ti,hwmods = "mmu_isp";
>>>>>>>> +		ti,#tlb-entries = <8>;
>>>>>>>> +		dma-window = <0 0xfffff000>;
>>>>>>>> +	};
>

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

On 02/26/2014 04:18 PM, Suman Anna wrote:
> Hi Laurent,
>
> On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
>> Hi Suman,
>>
>> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
>>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>>
>>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error
>>>>>>>>> back]
>>>>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28
>>>>>>>>> ++++++++++++
>>>>>>>>>   1 file changed, 28 insertions(+)
>>>>>>>>>   create mode 100644
>>>>>>>>>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>
>>>>>>>>> diff --git
>>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new
>>>>>>>>> file
>>>>>>>>> mode 100644
>>>>>>>>> index 0000000..116492d
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>>> +OMAP2+ IOMMU
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +- compatible : Should be one of,
>>>>>>>>> +        "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>>>>> +        "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>>>>> +        "ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU
>>>>>>>>> instance
>>>>>>>>> +- reg        : Address space for the configuration registers
>>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>>>>> +- dma-window : IOVA start address and length
>>>>>>>>
>>>>>>>> Isn't the dma window more of a system configuration property than a
>>>>>>>> hardware property ? How do you expect it to be set?
>>>>>>>
>>>>>>> We are setting it based on the addressable range for the MMU.
>>>>>>
>>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>>>>> support the full 4GB VA space. Why do you need to restrict it ?
>>>>>
>>>>> I should have rephrased it better when I said addressable range. While
>>>>> the MMUs are capable of programming the full 4GB space, there are some
>>>>> address ranges that are private from the processor view. This
>>>>> window is
>>>>> currently used to set the range for the omap-iovmm driver (which only
>>>>> OMAP3 ISP is using atm), and there is no point in allowing the
>>>>> omap-iovmm driver the full range when the processor could never
>>>>> reach/access those addresses.
>>>>
>>>> But the IOMMU VA space is from a device point of view, not from a CPU
>>>> point of view. Could you point me to where those private ranges are
>>>> documented, in order to understand the problem correctly ?
>>>
>>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
>>> by processor.
>>>
>>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
>>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
>>> addressable range starts from 0x11000000.
>>
>> OK, so it looks more like a property of the IOMMU master than a
>> property of
>> the IOMMU itself. It would be better to express it as such, but I
>> wonder how
>> that could be done, and if it would be worth it in this case.
>
> This property is currently solely used to configure the range for the
> omap-iovmm module, which were supplied through platform data in the
> non-DT case. I am wondering if the way to go here is to use
> iommu_domain_set_attr() and use the domain geometry values.

The other option is to supply these as driver match data, and switching
the compatible strings to identify the MMU instance precisely.

regards
Suman

>>
>> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
>> regarding the VA range they can address, should this property be at
>> least made
>> optional ?
>>
>>>>>>> We are reusing the existing defined property and it allows us to get
>>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
>>>>>>> iommu platform data.
>>>>>>>
>>>>>>>>> +Optional properties:
>>>>>>>>> +- ti,#tlb-entries : Number of entries in the translation
>>>>>>>>> look-aside
>>>>>>>>> buffer. +                    Should be either 8 or 32 (default:
>>>>>>>>> 32)
>>>>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
>>>>>>>>> throwing
>>>>>>>>> +                  back a bus error response on MMU faults.
>>>>>>>>
>>>>>>>> Do these features vary per IOMMU instance or per IOMMU model ?
>>>>>>>> In the
>>>>>>>> latter case they could be inferred from the compatible string by
>>>>>>>> the
>>>>>>>> driver without requiring them to be explicit in DT (whether you
>>>>>>>> want
>>>>>>>> to do so is left to you though).
>>>>>>>
>>>>>>> Well, these are fixed features given an IOMMU instance, like the
>>>>>>> OMAP3
>>>>>>> ISP is the only one that has 8 TLB entries, all the remaining
>>>>>>> ones have
>>>>>>> 32, and the IPU iommu instances are the only ones that support
>>>>>>> the bus
>>>>>>> error response back. I have no preference to any particular way, and
>>>>>>> sure the driver can infer these easily based on unique compatible
>>>>>>> strings per subsystem per SoC. I just happened to go with defining
>>>>>>> compatible strings per SoC, with the optional properties
>>>>>>> differentiating the fixed behavior between different IOMMU
>>>>>>> instances on
>>>>>>> that SoC. This is where I was looking for some inputs/guidance
>>>>>>> from the
>>>>>>> DT bindings maintainers on what is the preferred method.
>>>>>>
>>>>>> I think you've made the right choice. I wasn't sure whether those
>>>>>> parameters varied across IOMMU instances of compatible devices
>>>>>> (from a
>>>>>> compatible string point of view) or were constant. As they vary they
>>>>>> should be expressed in DT.
>>>>>
>>>>> Yeah, I wasn't sure if these qualify as features (as per
>>>>> Documentation/devicetree/bindings/ABI.txt section II.2).
>>>>>
>>>>> regards
>>>>> Suman
>>>>>
>>>>>>>>> +Example:
>>>>>>>>> +    /* OMAP3 ISP MMU */
>>>>>>>>> +    mmu_isp: mmu@480bd400 {
>>>>>>>>> +        compatible = "ti,omap2-iommu";
>>>>>>>>> +        reg = <0x480bd400 0x80>;
>>>>>>>>> +        interrupts = <24>;
>>>>>>>>> +        ti,hwmods = "mmu_isp";
>>>>>>>>> +        ti,#tlb-entries = <8>;
>>>>>>>>> +        dma-window = <0 0xfffff000>;
>>>>>>>>> +    };
>>
>

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Laurent Pinchart]

Hi Suman,

On Wednesday 26 February 2014 16:28:08 Suman Anna wrote:
> On 02/26/2014 04:18 PM, Suman Anna wrote:
> > On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
> >> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
> >>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
> >>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> >>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> >>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> >>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >>>>>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>>>>> 
> >>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
> >>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >>>>>>>>> 
> >>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error
> >>>>>>>>> back]
> >>>>>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
> >>>>>>>>> ---
> >>>>>>>>> 
> >>>>>>>>>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 +++++++++
> >>>>>>>>>  1 file changed, 28 insertions(+)
> >>>>>>>>>  create mode 100644
> >>>>>>>>>  Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>>>>> 
> >>>>>>>>> diff --git
> >>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new
> >>>>>>>>> file
> >>>>>>>>> mode 100644
> >>>>>>>>> index 0000000..116492d
> >>>>>>>>> --- /dev/null
> >>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>>>>> @@ -0,0 +1,28 @@
> >>>>>>>>> +OMAP2+ IOMMU
> >>>>>>>>> +
> >>>>>>>>> +Required properties:
> >>>>>>>>> +- compatible : Should be one of,
> >>>>>>>>> +        "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >>>>>>>>> +        "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >>>>>>>>> +        "ti,dra7-iommu" for DRA7xx IOMMU instances
> >>>>>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU
> >>>>>>>>> instance
> >>>>>>>>> +- reg        : Address space for the configuration registers
> >>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
> >>>>>>>>> +- dma-window : IOVA start address and length
> >>>>>>>> 
> >>>>>>>> Isn't the dma window more of a system configuration property than a
> >>>>>>>> hardware property ? How do you expect it to be set?
> >>>>>>> 
> >>>>>>> We are setting it based on the addressable range for the MMU.
> >>>>>> 
> >>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
> >>>>>> support the full 4GB VA space. Why do you need to restrict it ?
> >>>>> 
> >>>>> I should have rephrased it better when I said addressable range. While
> >>>>> the MMUs are capable of programming the full 4GB space, there are some
> >>>>> address ranges that are private from the processor view. This
> >>>>> window is currently used to set the range for the omap-iovmm driver
> >>>>> (which only OMAP3 ISP is using atm), and there is no point in allowing
> >>>>> the omap-iovmm driver the full range when the processor could never
> >>>>> reach/access those addresses.
> >>>> 
> >>>> But the IOMMU VA space is from a device point of view, not from a CPU
> >>>> point of view. Could you point me to where those private ranges are
> >>>> documented, in order to understand the problem correctly ?
> >>> 
> >>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
> >>> by processor.
> >>> 
> >>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
> >>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
> >>> addressable range starts from 0x11000000.
> >> 
> >> OK, so it looks more like a property of the IOMMU master than a
> >> property of the IOMMU itself. It would be better to express it as such,
> >> but I wonder how that could be done, and if it would be worth it in this
> >> case.
> > 
> > This property is currently solely used to configure the range for the
> > omap-iovmm module, which were supplied through platform data in the
> > non-DT case.

If I'm not mistaken omap-iovmm is something we want to get rid of. I know that 
the OMAP3 ISP driver is the only user of that module, and I've started working 
on fixing that, but I'm currently lacking time to complete the work.

Now, if we get rid of omap-iovmm, does that mean that the dma-window property 
won't need to be specified anymore ? If so, given that the only omap-iovmm 
user is the OMAP3 ISP driver, I would propose to drop the property and just 
hardcode the value.

Please let me know if there's something I've missed.

> > I am wondering if the way to go here is to use iommu_domain_set_attr() and
> > use the domain geometry values.
> 
> The other option is to supply these as driver match data, and switching
> the compatible strings to identify the MMU instance precisely.
> 
> regards
> Suman
> 
> >> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
> >> regarding the VA range they can address, should this property be at
> >> least made
> >> optional ?
> >> 
> >>>>>>> We are reusing the existing defined property and it allows us to get
> >>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
> >>>>>>> iommu platform data.

[snip]

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings

[Suman Anna]

Hi Laurent,

 >
> On Wednesday 26 February 2014 16:28:08 Suman Anna wrote:
>> On 02/26/2014 04:18 PM, Suman Anna wrote:
>>> On 02/26/2014 02:36 PM, Laurent Pinchart wrote:
>>>> On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
>>>>>> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>>>>>>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>>>>>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>>>>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>>>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>>>>
>>>>>>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error
>>>>>>>>>>> back]
>>>>>>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 +++++++++
>>>>>>>>>>>   1 file changed, 28 insertions(+)
>>>>>>>>>>>   create mode 100644
>>>>>>>>>>>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>>>
>>>>>>>>>>> diff --git
>>>>>>>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new
>>>>>>>>>>> file
>>>>>>>>>>> mode 100644
>>>>>>>>>>> index 0000000..116492d
>>>>>>>>>>> --- /dev/null
>>>>>>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>>>>>> @@ -0,0 +1,28 @@
>>>>>>>>>>> +OMAP2+ IOMMU
>>>>>>>>>>> +
>>>>>>>>>>> +Required properties:
>>>>>>>>>>> +- compatible : Should be one of,
>>>>>>>>>>> +        "ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>>>>>>> +        "ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>>>>>>> +        "ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>>>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU
>>>>>>>>>>> instance
>>>>>>>>>>> +- reg        : Address space for the configuration registers
>>>>>>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>>>>>>> +- dma-window : IOVA start address and length
>>>>>>>>>>
>>>>>>>>>> Isn't the dma window more of a system configuration property than a
>>>>>>>>>> hardware property ? How do you expect it to be set?
>>>>>>>>>
>>>>>>>>> We are setting it based on the addressable range for the MMU.
>>>>>>>>
>>>>>>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>>>>>>> support the full 4GB VA space. Why do you need to restrict it ?
>>>>>>>
>>>>>>> I should have rephrased it better when I said addressable range. While
>>>>>>> the MMUs are capable of programming the full 4GB space, there are some
>>>>>>> address ranges that are private from the processor view. This
>>>>>>> window is currently used to set the range for the omap-iovmm driver
>>>>>>> (which only OMAP3 ISP is using atm), and there is no point in allowing
>>>>>>> the omap-iovmm driver the full range when the processor could never
>>>>>>> reach/access those addresses.
>>>>>>
>>>>>> But the IOMMU VA space is from a device point of view, not from a CPU
>>>>>> point of view. Could you point me to where those private ranges are
>>>>>> documented, in order to understand the problem correctly ?
>>>>>
>>>>> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
>>>>> by processor.
>>>>>
>>>>> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
>>>>> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
>>>>> addressable range starts from 0x11000000.
>>>>
>>>> OK, so it looks more like a property of the IOMMU master than a
>>>> property of the IOMMU itself. It would be better to express it as such,
>>>> but I wonder how that could be done, and if it would be worth it in this
>>>> case.
>>>
>>> This property is currently solely used to configure the range for the
>>> omap-iovmm module, which were supplied through platform data in the
>>> non-DT case.
>
> If I'm not mistaken omap-iovmm is something we want to get rid of. I know that
> the OMAP3 ISP driver is the only user of that module, and I've started working
> on fixing that, but I'm currently lacking time to complete the work.
>
> Now, if we get rid of omap-iovmm, does that mean that the dma-window property
> won't need to be specified anymore ? If so, given that the only omap-iovmm
> user is the OMAP3 ISP driver, I would propose to drop the property and just
> hardcode the value.

Yeah, none of our OMAP4+ stacks use omap-iovmm, or similar dynamic 
reservation scheme at the moment. I am perfectly fine with dropping the 
property and hard-coding it in the driver with a note. DSP/Bridge has a 
similar usage (in dmm.c), but that code is localized within the driver.

Thanks for all the comments.

regards
Suman


>
> Please let me know if there's something I've missed.


>
>>> I am wondering if the way to go here is to use iommu_domain_set_attr() and
>>> use the domain geometry values.
>>
>> The other option is to supply these as driver match data, and switching
>> the compatible strings to identify the MMU instance precisely.
>>
>> regards
>> Suman
>>
>>>> As not all masters (the OMAP3 ISP doesn't for instance) have restrictions
>>>> regarding the VA range they can address, should this property be at
>>>> least made
>>>> optional ?
>>>>
>>>>>>>>> We are reusing the existing defined property and it allows us to get
>>>>>>>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
>>>>>>>>> iommu platform data.
>
> [snip]
>

[PATCHv2 04/16] iommu/omap: add devicetree support

[Suman Anna]

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

As OMAP2+ is moving to a full DT boot for all SoC families, commit
7ce93f3 "ARM: OMAP2+: Fix more missing data for omap3.dtsi file"
adds basic DT bits for OMAP3. But the driver is not yet converted,
so this will not work and driver will not be probed. Convert it!

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
[s-anna-l0cyMroinI0@public.gmane.org: dev_name adaptation and improved error checking]
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/omap-iommu.c |  5 +++++
 drivers/iommu/omap-iommu.c       | 41 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
index f6daae8..f1fab56 100644
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ b/arch/arm/mach-omap2/omap-iommu.c
@@ -10,6 +10,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/of.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
@@ -58,6 +59,10 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
 
 static int __init omap_iommu_init(void)
 {
+	/* If dtb is there, the devices will be created dynamically */
+	if (of_have_populated_dt())
+		return -ENODEV;
+
 	return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
 }
 /* must be ready before omap3isp is probed */
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 6272c36..4329ab1 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -23,6 +23,9 @@
 #include <linux/spinlock.h>
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
+#include <linux/of.h>
+#include <linux/of_iommu.h>
+#include <linux/of_irq.h>
 
 #include <asm/cacheflush.h>
 
@@ -937,20 +940,41 @@ static int omap_iommu_probe(struct platform_device *pdev)
 {
 	int err = -ENODEV;
 	int irq;
+	size_t len;
 	struct omap_iommu *obj;
 	struct resource *res;
 	struct iommu_platform_data *pdata = pdev->dev.platform_data;
+	struct device_node *of = pdev->dev.of_node;
 
 	obj = devm_kzalloc(&pdev->dev, sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
 	if (!obj)
 		return -ENOMEM;
 
-	obj->nr_tlb_entries = pdata->nr_tlb_entries;
-	obj->name = pdata->name;
+	if (of) {
+		obj->name = dev_name(&pdev->dev);
+		obj->nr_tlb_entries = 32;
+		err = of_property_read_u32(of, "ti,#tlb-entries",
+					   &obj->nr_tlb_entries);
+		if (err && err != -EINVAL)
+			return err;
+		if (obj->nr_tlb_entries != 32 && obj->nr_tlb_entries != 8)
+			return -EINVAL;
+		err = of_get_dma_window(of, NULL, 0, NULL, &obj->da_start,
+					&len);
+		if (err != 0)
+			return err;
+		obj->da_end = obj->da_start + len;
+	} else {
+		obj->nr_tlb_entries = pdata->nr_tlb_entries;
+		obj->name = pdata->name;
+		obj->da_start = pdata->da_start;
+		obj->da_end = pdata->da_end;
+	}
+	if (obj->da_end <= obj->da_start)
+		return -EINVAL;
+
 	obj->dev = &pdev->dev;
 	obj->ctx = (void *)obj + sizeof(*obj);
-	obj->da_start = pdata->da_start;
-	obj->da_end = pdata->da_end;
 
 	spin_lock_init(&obj->iommu_lock);
 	mutex_init(&obj->mmap_lock);
@@ -991,11 +1015,20 @@ static int omap_iommu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct of_device_id omap_iommu_of_match[] = {
+	{ .compatible = "ti,omap2-iommu" },
+	{ .compatible = "ti,omap4-iommu" },
+	{ .compatible = "ti,dra7-iommu"	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap_iommu_of_match);
+
 static struct platform_driver omap_iommu_driver = {
 	.probe	= omap_iommu_probe,
 	.remove	= omap_iommu_remove,
 	.driver	= {
 		.name	= "omap-iommu",
+		.of_match_table = of_match_ptr(omap_iommu_of_match),
 	},
 };
 
-- 
1.8.5.3

Re: [PATCHv2 04/16] iommu/omap: add devicetree support

[Tony Lindgren]

* Suman Anna <s-anna@ti.com> [140213 10:19]:
> From: Florian Vaussard <florian.vaussard@epfl.ch>
> 
> As OMAP2+ is moving to a full DT boot for all SoC families, commit
> 7ce93f3 "ARM: OMAP2+: Fix more missing data for omap3.dtsi file"
> adds basic DT bits for OMAP3. But the driver is not yet converted,
> so this will not work and driver will not be probed. Convert it!
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> [s-anna@ti.com: dev_name adaptation and improved error checking]
> Signed-off-by: Suman Anna <s-anna@ti.com>

Best that this gets merged along with the other iommu patches, so
for the arch/arm/*omap* parts:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/mach-omap2/omap-iommu.c |  5 +++++
>  drivers/iommu/omap-iommu.c       | 41 ++++++++++++++++++++++++++++++++++++----
>  2 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
> index f6daae8..f1fab56 100644
> --- a/arch/arm/mach-omap2/omap-iommu.c
> +++ b/arch/arm/mach-omap2/omap-iommu.c
> @@ -10,6 +10,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/of.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/err.h>
> @@ -58,6 +59,10 @@ static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
>  
>  static int __init omap_iommu_init(void)
>  {
> +	/* If dtb is there, the devices will be created dynamically */
> +	if (of_have_populated_dt())
> +		return -ENODEV;
> +
>  	return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
>  }
>  /* must be ready before omap3isp is probed */
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index 6272c36..4329ab1 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -23,6 +23,9 @@
>  #include <linux/spinlock.h>
>  #include <linux/io.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/of_iommu.h>
> +#include <linux/of_irq.h>
>  
>  #include <asm/cacheflush.h>
>  
> @@ -937,20 +940,41 @@ static int omap_iommu_probe(struct platform_device *pdev)
>  {
>  	int err = -ENODEV;
>  	int irq;
> +	size_t len;
>  	struct omap_iommu *obj;
>  	struct resource *res;
>  	struct iommu_platform_data *pdata = pdev->dev.platform_data;
> +	struct device_node *of = pdev->dev.of_node;
>  
>  	obj = devm_kzalloc(&pdev->dev, sizeof(*obj) + MMU_REG_SIZE, GFP_KERNEL);
>  	if (!obj)
>  		return -ENOMEM;
>  
> -	obj->nr_tlb_entries = pdata->nr_tlb_entries;
> -	obj->name = pdata->name;
> +	if (of) {
> +		obj->name = dev_name(&pdev->dev);
> +		obj->nr_tlb_entries = 32;
> +		err = of_property_read_u32(of, "ti,#tlb-entries",
> +					   &obj->nr_tlb_entries);
> +		if (err && err != -EINVAL)
> +			return err;
> +		if (obj->nr_tlb_entries != 32 && obj->nr_tlb_entries != 8)
> +			return -EINVAL;
> +		err = of_get_dma_window(of, NULL, 0, NULL, &obj->da_start,
> +					&len);
> +		if (err != 0)
> +			return err;
> +		obj->da_end = obj->da_start + len;
> +	} else {
> +		obj->nr_tlb_entries = pdata->nr_tlb_entries;
> +		obj->name = pdata->name;
> +		obj->da_start = pdata->da_start;
> +		obj->da_end = pdata->da_end;
> +	}
> +	if (obj->da_end <= obj->da_start)
> +		return -EINVAL;
> +
>  	obj->dev = &pdev->dev;
>  	obj->ctx = (void *)obj + sizeof(*obj);
> -	obj->da_start = pdata->da_start;
> -	obj->da_end = pdata->da_end;
>  
>  	spin_lock_init(&obj->iommu_lock);
>  	mutex_init(&obj->mmap_lock);
> @@ -991,11 +1015,20 @@ static int omap_iommu_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct of_device_id omap_iommu_of_match[] = {
> +	{ .compatible = "ti,omap2-iommu" },
> +	{ .compatible = "ti,omap4-iommu" },
> +	{ .compatible = "ti,dra7-iommu"	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, omap_iommu_of_match);
> +
>  static struct platform_driver omap_iommu_driver = {
>  	.probe	= omap_iommu_probe,
>  	.remove	= omap_iommu_remove,
>  	.driver	= {
>  		.name	= "omap-iommu",
> +		.of_match_table = of_match_ptr(omap_iommu_of_match),
>  	},
>  };
>  
> -- 
> 1.8.5.3
> 

[PATCHv2 05/16] iommu/omap: enable bus-error back on supported iommus

[Suman Anna]

The remoteproc MMUs in OMAP4+ SoCs have some additional debug
registers that can give out the PC value in addition to the
MMU fault address. The PC value can be extracted properly only
on the DSP cores, and is not available on the ARM processors
within the IPU sub-systems. Instead, the MMUs have been enhanced
to throw a bus-error response back to the IPU processors.

This functionality is programmable through the MMU_GP_REG register.
The cores are simply stalled if the MMU_GP_REG.BUS_ERR_BACK_EN bit
is not set. When set, a bus-error exception is raised allowing the
processor to handle it as a bus fault and provide additional debug
information. This feature is turned on by default by the driver on
iommus supporting it.

Signed-off-by: Subramaniam Chanderashekarapuram <subramaniam.ca-l0cyMroinI0@public.gmane.org>
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 drivers/iommu/omap-iommu.c  | 2 ++
 drivers/iommu/omap-iommu.h  | 5 +++++
 drivers/iommu/omap-iommu2.c | 3 +++
 3 files changed, 10 insertions(+)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 4329ab1..e64025a 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -964,6 +964,8 @@ static int omap_iommu_probe(struct platform_device *pdev)
 		if (err != 0)
 			return err;
 		obj->da_end = obj->da_start + len;
+		if (of_find_property(of, "ti,iommu-bus-err-back", NULL))
+			obj->has_bus_err_back = MMU_GP_REG_BUS_ERR_BACK_EN;
 	} else {
 		obj->nr_tlb_entries = pdata->nr_tlb_entries;
 		obj->name = pdata->name;
diff --git a/drivers/iommu/omap-iommu.h b/drivers/iommu/omap-iommu.h
index 1200842..ea920c3 100644
--- a/drivers/iommu/omap-iommu.h
+++ b/drivers/iommu/omap-iommu.h
@@ -52,6 +52,8 @@ struct omap_iommu {
 	void *ctx; /* iommu context: registres saved area */
 	u32 da_start;
 	u32 da_end;
+
+	int has_bus_err_back;
 };
 
 struct cr_regs {
@@ -130,6 +132,7 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev)
 #define MMU_READ_CAM		0x68
 #define MMU_READ_RAM		0x6c
 #define MMU_EMU_FAULT_AD	0x70
+#define MMU_GP_REG		0x88
 
 #define MMU_REG_SIZE		256
 
@@ -163,6 +166,8 @@ static inline struct omap_iommu *dev_to_omap_iommu(struct device *dev)
 #define MMU_RAM_MIXED_MASK	(1 << MMU_RAM_MIXED_SHIFT)
 #define MMU_RAM_MIXED		MMU_RAM_MIXED_MASK
 
+#define MMU_GP_REG_BUS_ERR_BACK_EN	0x1
+
 /*
  * utilities for super page(16MB, 1MB, 64KB and 4KB)
  */
diff --git a/drivers/iommu/omap-iommu2.c b/drivers/iommu/omap-iommu2.c
index d745094..5e1ea3b 100644
--- a/drivers/iommu/omap-iommu2.c
+++ b/drivers/iommu/omap-iommu2.c
@@ -98,6 +98,9 @@ static int omap2_iommu_enable(struct omap_iommu *obj)
 
 	iommu_write_reg(obj, pa, MMU_TTB);
 
+	if (obj->has_bus_err_back)
+		iommu_write_reg(obj, MMU_GP_REG_BUS_ERR_BACK_EN, MMU_GP_REG);
+
 	__iommu_set_twl(obj, true);
 
 	return 0;
-- 
1.8.5.3

[PATCHv2 06/16] iommu/omap: allocate archdata on the fly for DT-based devices

[Suman Anna]

From: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

The OMAP IOMMU driver locates the IOMMU associated to a device using the
IOMMU name stored in the device archdata iommu field. That field is
expected to be populated by platform code and is left unset for DT-based
devices. This results in a crash when the IOMMU driver attaches a domain
to a device.

Fix this by allocating the archdata iommu structure when devices are
added and freeing when they are removed. Devices without an OF node, and
devices without an iommus property in their OF node are ignored. The
iommu name is initialized from the IOMMU device node name.

This should be simplified when removing non-DT support completely from
the IOMMU users as the IOMMU name won't be needed anymore, and the
IOMMU device pointer could then be stored in the archdata iommu field
directly.

Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
[s-anna-l0cyMroinI0@public.gmane.org: updated to use device name instead of OF name]
Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 drivers/iommu/omap-iommu.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index e64025a..f6afe8f 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1256,6 +1256,49 @@ static int omap_iommu_domain_has_cap(struct iommu_domain *domain,
 	return 0;
 }
 
+static int omap_iommu_add_device(struct device *dev)
+{
+	struct omap_iommu_arch_data *arch_data;
+	struct device_node *np;
+
+	/*
+	 * Allocate the archdata iommu structure for DT-based devices.
+	 *
+	 * TODO: Simplify this when removing non-DT support completely from the
+	 * IOMMU users.
+	 */
+	if (!dev->of_node)
+		return 0;
+
+	np = of_parse_phandle(dev->of_node, "iommus", 0);
+	if (!np)
+		return 0;
+
+	arch_data = kzalloc(sizeof(*arch_data), GFP_KERNEL);
+	if (!arch_data) {
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	arch_data->name = kstrdup(dev_name(dev), GFP_KERNEL);
+	dev->archdata.iommu = arch_data;
+
+	of_node_put(np);
+
+	return 0;
+}
+
+static void omap_iommu_remove_device(struct device *dev)
+{
+	struct omap_iommu_arch_data *arch_data = dev->archdata.iommu;
+
+	if (!dev->of_node || !arch_data)
+		return;
+
+	kfree(arch_data->name);
+	kfree(arch_data);
+}
+
 static struct iommu_ops omap_iommu_ops = {
 	.domain_init	= omap_iommu_domain_init,
 	.domain_destroy	= omap_iommu_domain_destroy,
@@ -1265,6 +1308,8 @@ static struct iommu_ops omap_iommu_ops = {
 	.unmap		= omap_iommu_unmap,
 	.iova_to_phys	= omap_iommu_iova_to_phys,
 	.domain_has_cap	= omap_iommu_domain_has_cap,
+	.add_device	= omap_iommu_add_device,
+	.remove_device	= omap_iommu_remove_device,
 	.pgsize_bitmap	= OMAP_IOMMU_PGSIZES,
 };
 
-- 
1.8.5.3

[PATCHv2 07/16] iommu/omap: allow enable/disable even without pdata

[Suman Anna]

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

When booting with a devicetree, no platform data is provided.
Do not prematurely exit iommu_enable() and iommu_disable() in
such a case.

Note: As OMAP do not yet has a proper reset controller driver,
IOMMUs requiring a reset signal should use pdata-quirks as a
transitional solution.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
---
 drivers/iommu/omap-iommu.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index f6afe8f..7672eb4 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -149,13 +149,10 @@ static int iommu_enable(struct omap_iommu *obj)
 	struct platform_device *pdev = to_platform_device(obj->dev);
 	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	if (!pdata)
-		return -EINVAL;
-
 	if (!arch_iommu)
 		return -ENODEV;
 
-	if (pdata->deassert_reset) {
+	if (pdata && pdata->deassert_reset) {
 		err = pdata->deassert_reset(pdev, pdata->reset_name);
 		if (err) {
 			dev_err(obj->dev, "deassert_reset failed: %d\n", err);
@@ -175,14 +172,11 @@ static void iommu_disable(struct omap_iommu *obj)
 	struct platform_device *pdev = to_platform_device(obj->dev);
 	struct iommu_platform_data *pdata = pdev->dev.platform_data;
 
-	if (!pdata)
-		return;
-
 	arch_iommu->disable(obj);
 
 	pm_runtime_put_sync(obj->dev);
 
-	if (pdata->assert_reset)
+	if (pdata && pdata->assert_reset)
 		pdata->assert_reset(pdev, pdata->reset_name);
 }
 
-- 
1.8.5.3

Re: [PATCHv2 07/16] iommu/omap: allow enable/disable even without pdata

[Laurent Pinchart]

Hi Suman,

Thank you for the patch.

On Thursday 13 February 2014 12:15:38 Suman Anna wrote:
> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> 
> When booting with a devicetree, no platform data is provided.
> Do not prematurely exit iommu_enable() and iommu_disable() in
> such a case.
> 
> Note: As OMAP do not yet has a proper reset controller driver,
> IOMMUs requiring a reset signal should use pdata-quirks as a
> transitional solution.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

This looks good to me, but you should move this patch before 04/16, otherwise 
you'll break bisection.

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
>  drivers/iommu/omap-iommu.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> index f6afe8f..7672eb4 100644
> --- a/drivers/iommu/omap-iommu.c
> +++ b/drivers/iommu/omap-iommu.c
> @@ -149,13 +149,10 @@ static int iommu_enable(struct omap_iommu *obj)
>  	struct platform_device *pdev = to_platform_device(obj->dev);
>  	struct iommu_platform_data *pdata = pdev->dev.platform_data;
> 
> -	if (!pdata)
> -		return -EINVAL;
> -
>  	if (!arch_iommu)
>  		return -ENODEV;
> 
> -	if (pdata->deassert_reset) {
> +	if (pdata && pdata->deassert_reset) {
>  		err = pdata->deassert_reset(pdev, pdata->reset_name);
>  		if (err) {
>  			dev_err(obj->dev, "deassert_reset failed: %d\n", err);
> @@ -175,14 +172,11 @@ static void iommu_disable(struct omap_iommu *obj)
>  	struct platform_device *pdev = to_platform_device(obj->dev);
>  	struct iommu_platform_data *pdata = pdev->dev.platform_data;
> 
> -	if (!pdata)
> -		return;
> -
>  	arch_iommu->disable(obj);
> 
>  	pm_runtime_put_sync(obj->dev);
> 
> -	if (pdata->assert_reset)
> +	if (pdata && pdata->assert_reset)
>  		pdata->assert_reset(pdev, pdata->reset_name);
>  }

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 07/16] iommu/omap: allow enable/disable even without pdata

[Suman Anna]

Hi Laurent,

On 02/25/2014 03:15 PM, Laurent Pinchart wrote:
> Hi Suman,
>
> Thank you for the patch.
>
> On Thursday 13 February 2014 12:15:38 Suman Anna wrote:
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> When booting with a devicetree, no platform data is provided.
>> Do not prematurely exit iommu_enable() and iommu_disable() in
>> such a case.
>>
>> Note: As OMAP do not yet has a proper reset controller driver,
>> IOMMUs requiring a reset signal should use pdata-quirks as a
>> transitional solution.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>
> This looks good to me, but you should move this patch before 04/16, otherwise
> you'll break bisection.

OK, I see the point w.r.t the OMAP3 ISP node. I will move it to before 
03/16 (the DT bindings patch), it shouldn't make any difference.

regards
Suman

>
> Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
>
>> ---
>>   drivers/iommu/omap-iommu.c | 10 ++--------
>>   1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
>> index f6afe8f..7672eb4 100644
>> --- a/drivers/iommu/omap-iommu.c
>> +++ b/drivers/iommu/omap-iommu.c
>> @@ -149,13 +149,10 @@ static int iommu_enable(struct omap_iommu *obj)
>>   	struct platform_device *pdev = to_platform_device(obj->dev);
>>   	struct iommu_platform_data *pdata = pdev->dev.platform_data;
>>
>> -	if (!pdata)
>> -		return -EINVAL;
>> -
>>   	if (!arch_iommu)
>>   		return -ENODEV;
>>
>> -	if (pdata->deassert_reset) {
>> +	if (pdata && pdata->deassert_reset) {
>>   		err = pdata->deassert_reset(pdev, pdata->reset_name);
>>   		if (err) {
>>   			dev_err(obj->dev, "deassert_reset failed: %d\n", err);
>> @@ -175,14 +172,11 @@ static void iommu_disable(struct omap_iommu *obj)
>>   	struct platform_device *pdev = to_platform_device(obj->dev);
>>   	struct iommu_platform_data *pdata = pdev->dev.platform_data;
>>
>> -	if (!pdata)
>> -		return;
>> -
>>   	arch_iommu->disable(obj);
>>
>>   	pm_runtime_put_sync(obj->dev);
>>
>> -	if (pdata->assert_reset)
>> +	if (pdata && pdata->assert_reset)
>>   		pdata->assert_reset(pdev, pdata->reset_name);
>>   }
>

[PATCHv2 09/16] ARM: OMAP2+: change the ISP device archdata MMU name

[Suman Anna]

The IOMMU DT adaptation support uses the device name instead
of an iommu object name. The iommu object names should eventually
vanish when all the IOMMU users have been converted to DT nodes.

NOTE: This change is not compatible with legacy boots, but OMAP3
is expected to be DT-boot only going forward.

Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/devices.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 0dd6398..3bf0452 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -224,7 +224,7 @@ static struct platform_device omap3isp_device = {
 };
 
 static struct omap_iommu_arch_data omap3_isp_iommu = {
-	.name = "mmu_isp",
+	.name = "480bd400.mmu",
 };
 
 int omap3_init_camera(struct isp_platform_data *pdata)
-- 
1.8.5.3

[PATCHv2 10/16] ARM: OMAP2+: use pdata quirks for iommu reset lines

[Suman Anna]

The OMAP iommu driver performs the reset management for the
iommu instances in processor sub-systems using the omap_device
API which are currently supplied as platform data ops. Use pdata
quirks to maintain the functionality as the OMAP iommu driver
gets converted to use DT nodes, until the reset portions are
decoupled from omap_hwmod/omap_device into a separate reset
driver.

This patch adds the pdata quirks for the reset management of
iommus within the DSP (OMAP3 & OMAP4) and IPU subsystems (OMAP4).

Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/pdata-quirks.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 3d5b24d..74e094a 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -16,12 +16,14 @@
 #include <linux/wl12xx.h>
 
 #include <linux/platform_data/pinctrl-single.h>
+#include <linux/platform_data/iommu-omap.h>
 
 #include "am35xx.h"
 #include "common.h"
 #include "common-board-devices.h"
 #include "dss-common.h"
 #include "control.h"
+#include "omap_device.h"
 
 struct pdata_init {
 	const char *compatible;
@@ -92,6 +94,12 @@ static void __init hsmmc2_internal_input_clk(void)
 	omap_ctrl_writel(reg, OMAP343X_CONTROL_DEVCONF1);
 }
 
+static struct iommu_platform_data omap3_iommu_pdata = {
+	.reset_name = "mmu",
+	.assert_reset = omap_device_assert_hardreset,
+	.deassert_reset = omap_device_deassert_hardreset,
+};
+
 static int omap3_sbc_t3730_twl_callback(struct device *dev,
 					   unsigned gpio,
 					   unsigned ngpio)
@@ -185,6 +193,12 @@ static void __init omap4_panda_legacy_init(void)
 	legacy_init_ehci_clk("auxclk3_ck");
 	legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
 }
+
+static struct iommu_platform_data omap4_iommu_pdata = {
+	.reset_name = "mmu_cache",
+	.assert_reset = omap_device_assert_hardreset,
+	.deassert_reset = omap_device_deassert_hardreset,
+};
 #endif
 
 #ifdef CONFIG_SOC_OMAP5
@@ -240,6 +254,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
 #ifdef CONFIG_ARCH_OMAP3
 	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002030, "48002030.pinmux", &pcs_pdata),
 	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002a00, "48002a00.pinmux", &pcs_pdata),
+	OF_DEV_AUXDATA("ti,omap2-iommu", 0x5d000000, "5d000000.mmu",
+		       &omap3_iommu_pdata),
 	/* Only on am3517 */
 	OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c030000, "davinci_mdio.0", NULL),
 	OF_DEV_AUXDATA("ti,am3517-emac", 0x5c000000, "davinci_emac.0",
@@ -248,6 +264,10 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
 #ifdef CONFIG_ARCH_OMAP4
 	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
 	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
+	OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
+		       &omap4_iommu_pdata),
+	OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
+		       &omap4_iommu_pdata),
 #endif
 	{ /* sentinel */ },
 };
-- 
1.8.5.3

Re: [PATCHv2 10/16] ARM: OMAP2+: use pdata quirks for iommu reset lines

[Tony Lindgren]

* Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
> The OMAP iommu driver performs the reset management for the
> iommu instances in processor sub-systems using the omap_device
> API which are currently supplied as platform data ops. Use pdata
> quirks to maintain the functionality as the OMAP iommu driver
> gets converted to use DT nodes, until the reset portions are
> decoupled from omap_hwmod/omap_device into a separate reset
> driver.
> 
> This patch adds the pdata quirks for the reset management of
> iommus within the DSP (OMAP3 & OMAP4) and IPU subsystems (OMAP4).
> 
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>

Looks OK, but I suggest you separate out the remaining patches in
this series into another clean-up series. Then the clean-up series
can be merged later on as these have a good chance of conflicting
with other stuff.

Tony

> ---
>  arch/arm/mach-omap2/pdata-quirks.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
> index 3d5b24d..74e094a 100644
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -16,12 +16,14 @@
>  #include <linux/wl12xx.h>
>  
>  #include <linux/platform_data/pinctrl-single.h>
> +#include <linux/platform_data/iommu-omap.h>
>  
>  #include "am35xx.h"
>  #include "common.h"
>  #include "common-board-devices.h"
>  #include "dss-common.h"
>  #include "control.h"
> +#include "omap_device.h"
>  
>  struct pdata_init {
>  	const char *compatible;
> @@ -92,6 +94,12 @@ static void __init hsmmc2_internal_input_clk(void)
>  	omap_ctrl_writel(reg, OMAP343X_CONTROL_DEVCONF1);
>  }
>  
> +static struct iommu_platform_data omap3_iommu_pdata = {
> +	.reset_name = "mmu",
> +	.assert_reset = omap_device_assert_hardreset,
> +	.deassert_reset = omap_device_deassert_hardreset,
> +};
> +
>  static int omap3_sbc_t3730_twl_callback(struct device *dev,
>  					   unsigned gpio,
>  					   unsigned ngpio)
> @@ -185,6 +193,12 @@ static void __init omap4_panda_legacy_init(void)
>  	legacy_init_ehci_clk("auxclk3_ck");
>  	legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
>  }
> +
> +static struct iommu_platform_data omap4_iommu_pdata = {
> +	.reset_name = "mmu_cache",
> +	.assert_reset = omap_device_assert_hardreset,
> +	.deassert_reset = omap_device_deassert_hardreset,
> +};
>  #endif
>  
>  #ifdef CONFIG_SOC_OMAP5
> @@ -240,6 +254,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>  #ifdef CONFIG_ARCH_OMAP3
>  	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002030, "48002030.pinmux", &pcs_pdata),
>  	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002a00, "48002a00.pinmux", &pcs_pdata),
> +	OF_DEV_AUXDATA("ti,omap2-iommu", 0x5d000000, "5d000000.mmu",
> +		       &omap3_iommu_pdata),
>  	/* Only on am3517 */
>  	OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c030000, "davinci_mdio.0", NULL),
>  	OF_DEV_AUXDATA("ti,am3517-emac", 0x5c000000, "davinci_emac.0",
> @@ -248,6 +264,10 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>  #ifdef CONFIG_ARCH_OMAP4
>  	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
>  	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
> +	OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
> +		       &omap4_iommu_pdata),
> +	OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
> +		       &omap4_iommu_pdata),
>  #endif
>  	{ /* sentinel */ },
>  };
> -- 
> 1.8.5.3
> 

Re: [PATCHv2 10/16] ARM: OMAP2+: use pdata quirks for iommu reset lines

[Suman Anna]

Hi Tony,

On 02/26/2014 11:17 AM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
>> The OMAP iommu driver performs the reset management for the
>> iommu instances in processor sub-systems using the omap_device
>> API which are currently supplied as platform data ops. Use pdata
>> quirks to maintain the functionality as the OMAP iommu driver
>> gets converted to use DT nodes, until the reset portions are
>> decoupled from omap_hwmod/omap_device into a separate reset
>> driver.
>>
>> This patch adds the pdata quirks for the reset management of
>> iommus within the DSP (OMAP3 & OMAP4) and IPU subsystems (OMAP4).
>>
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>
> Looks OK, but I suggest you separate out the remaining patches in
> this series into another clean-up series. Then the clean-up series
> can be merged later on as these have a good chance of conflicting
> with other stuff.

Only patches 14 through 16 are cleanup patches. Patches 12 and 13 are 
adding OMAP5 functionality, and Patch11 is fixing up OMAP3 IVA. I would 
have to drop Patch14 and Patch16 until OMAP3 is completely DT, so will 
drop them for now. Let me know if you want me to split out the remaining 
applicable patches that are in arch/arm/ into a separate series.

regards
Suman

>
> Tony
>
>> ---
>>   arch/arm/mach-omap2/pdata-quirks.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
>> index 3d5b24d..74e094a 100644
>> --- a/arch/arm/mach-omap2/pdata-quirks.c
>> +++ b/arch/arm/mach-omap2/pdata-quirks.c
>> @@ -16,12 +16,14 @@
>>   #include <linux/wl12xx.h>
>>
>>   #include <linux/platform_data/pinctrl-single.h>
>> +#include <linux/platform_data/iommu-omap.h>
>>
>>   #include "am35xx.h"
>>   #include "common.h"
>>   #include "common-board-devices.h"
>>   #include "dss-common.h"
>>   #include "control.h"
>> +#include "omap_device.h"
>>
>>   struct pdata_init {
>>   	const char *compatible;
>> @@ -92,6 +94,12 @@ static void __init hsmmc2_internal_input_clk(void)
>>   	omap_ctrl_writel(reg, OMAP343X_CONTROL_DEVCONF1);
>>   }
>>
>> +static struct iommu_platform_data omap3_iommu_pdata = {
>> +	.reset_name = "mmu",
>> +	.assert_reset = omap_device_assert_hardreset,
>> +	.deassert_reset = omap_device_deassert_hardreset,
>> +};
>> +
>>   static int omap3_sbc_t3730_twl_callback(struct device *dev,
>>   					   unsigned gpio,
>>   					   unsigned ngpio)
>> @@ -185,6 +193,12 @@ static void __init omap4_panda_legacy_init(void)
>>   	legacy_init_ehci_clk("auxclk3_ck");
>>   	legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
>>   }
>> +
>> +static struct iommu_platform_data omap4_iommu_pdata = {
>> +	.reset_name = "mmu_cache",
>> +	.assert_reset = omap_device_assert_hardreset,
>> +	.deassert_reset = omap_device_deassert_hardreset,
>> +};
>>   #endif
>>
>>   #ifdef CONFIG_SOC_OMAP5
>> @@ -240,6 +254,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>>   #ifdef CONFIG_ARCH_OMAP3
>>   	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002030, "48002030.pinmux", &pcs_pdata),
>>   	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002a00, "48002a00.pinmux", &pcs_pdata),
>> +	OF_DEV_AUXDATA("ti,omap2-iommu", 0x5d000000, "5d000000.mmu",
>> +		       &omap3_iommu_pdata),
>>   	/* Only on am3517 */
>>   	OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c030000, "davinci_mdio.0", NULL),
>>   	OF_DEV_AUXDATA("ti,am3517-emac", 0x5c000000, "davinci_emac.0",
>> @@ -248,6 +264,10 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>>   #ifdef CONFIG_ARCH_OMAP4
>>   	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
>>   	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
>> +	OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
>> +		       &omap4_iommu_pdata),
>> +	OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
>> +		       &omap4_iommu_pdata),
>>   #endif
>>   	{ /* sentinel */ },
>>   };
>> --
>> 1.8.5.3
>>

[PATCHv2 11/16] ARM: OMAP3: fix iva mmu programming issues

[Suman Anna]

The IVA MMU is not functional when used through the hwmod and
omap_device layers. Add fixes to clockdomain and hwmod data
to have it functional. The hwmod changes are needed to enable
the clock, and the SWSUP change is needed to wakeup the domain
because the power domain is programmed to be in RET, and there
is no automatic power domain switching to ON.

Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/clockdomains3xxx_data.c | 2 +-
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c  | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/clockdomains3xxx_data.c b/arch/arm/mach-omap2/clockdomains3xxx_data.c
index e6b91e5..f03dc97 100644
--- a/arch/arm/mach-omap2/clockdomains3xxx_data.c
+++ b/arch/arm/mach-omap2/clockdomains3xxx_data.c
@@ -247,7 +247,7 @@ static struct clockdomain neon_clkdm = {
 static struct clockdomain iva2_clkdm = {
 	.name		= "iva2_clkdm",
 	.pwrdm		= { .name = "iva2_pwrdm" },
-	.flags		= CLKDM_CAN_HWSUP_SWSUP,
+	.flags		= CLKDM_CAN_SWSUP,
 	.dep_bit	= OMAP3430_PM_WKDEP_MPU_EN_IVA2_SHIFT,
 	.wkdep_srcs	= iva2_wkdeps,
 	.clktrctrl_mask = OMAP3430_CLKTRCTRL_IVA2_MASK,
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 81dd071..9c7e23a 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3068,12 +3068,16 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
 	.name		= "mmu_iva",
 	.class		= &omap3xxx_mmu_hwmod_class,
 	.mpu_irqs	= omap3xxx_mmu_iva_irqs,
+	.clkdm_name	= "iva2_clkdm",
 	.rst_lines	= omap3xxx_mmu_iva_resets,
 	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_mmu_iva_resets),
 	.main_clk	= "iva2_ck",
 	.prcm = {
 		.omap2 = {
 			.module_offs = OMAP3430_IVA2_MOD,
+			.module_bit = OMAP3430_CM_FCLKEN_IVA2_EN_IVA2_SHIFT,
+			.idlest_reg_id = 1,
+			.idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
 		},
 	},
 	.dev_attr	= &mmu_iva_dev_attr,
-- 
1.8.5.3

[PATCHv2 12/16] ARM: OMAP5: hwmod data: add mmu data for ipu & dsp

[Suman Anna]

A new MMU hwmod class and data structures are created
to represent the MMUs within the IPU and DSP processor
subsystems in OMAP5. The MMUs in OMAP5 are identical to
those in OMAP4.

Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
---
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c | 83 ++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
index e297d62..8923172 100644
--- a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
@@ -1122,6 +1122,71 @@ static struct omap_hwmod omap54xx_mmc5_hwmod = {
 };
 
 /*
+ * 'mmu' class
+ * The memory management unit performs virtual to physical address translation
+ * for its requestors.
+ */
+
+static struct omap_hwmod_class_sysconfig omap54xx_mmu_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.syss_offs	= 0x0014,
+	.sysc_flags	= (SYSC_HAS_AUTOIDLE | SYSC_HAS_CLOCKACTIVITY |
+			   SYSC_HAS_SIDLEMODE | SYSC_HAS_SOFTRESET |
+			   SYSS_HAS_RESET_STATUS),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap54xx_mmu_hwmod_class = {
+	.name = "mmu",
+	.sysc = &omap54xx_mmu_sysc,
+};
+
+static struct omap_hwmod_rst_info omap54xx_mmu_dsp_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 1 },
+};
+
+static struct omap_hwmod omap54xx_mmu_dsp_hwmod = {
+	.name		= "mmu_dsp",
+	.class		= &omap54xx_mmu_hwmod_class,
+	.clkdm_name	= "dsp_clkdm",
+	.rst_lines	= omap54xx_mmu_dsp_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap54xx_mmu_dsp_resets),
+	.main_clk	= "dpll_iva_h11x2_ck",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = OMAP54XX_CM_DSP_DSP_CLKCTRL_OFFSET,
+			.rstctrl_offs = OMAP54XX_RM_DSP_RSTCTRL_OFFSET,
+			.context_offs = OMAP54XX_RM_DSP_DSP_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		},
+	},
+};
+
+/* mmu ipu */
+static struct omap_hwmod_rst_info omap54xx_mmu_ipu_resets[] = {
+	{ .name = "mmu_cache", .rst_shift = 2 },
+};
+
+static struct omap_hwmod omap54xx_mmu_ipu_hwmod = {
+	.name		= "mmu_ipu",
+	.class		= &omap54xx_mmu_hwmod_class,
+	.clkdm_name	= "ipu_clkdm",
+	.rst_lines	= omap54xx_mmu_ipu_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap54xx_mmu_ipu_resets),
+	.main_clk	= "dpll_core_h22x2_ck",
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = OMAP54XX_CM_IPU_IPU_CLKCTRL_OFFSET,
+			.rstctrl_offs = OMAP54XX_RM_IPU_RSTCTRL_OFFSET,
+			.context_offs = OMAP54XX_RM_IPU_IPU_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		},
+	},
+};
+
+/*
  * 'mpu' class
  * mpu sub-system
  */
@@ -1763,6 +1828,14 @@ static struct omap_hwmod_ocp_if omap54xx_l4_cfg__l3_main_1 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* l4_cfg -> mmu_dsp */
+static struct omap_hwmod_ocp_if omap54xx_l4_cfg__mmu_dsp = {
+	.master		= &omap54xx_l4_cfg_hwmod,
+	.slave		= &omap54xx_mmu_dsp_hwmod,
+	.clk		= "l4_root_clk_div",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 /* mpu -> l3_main_1 */
 static struct omap_hwmod_ocp_if omap54xx_mpu__l3_main_1 = {
 	.master		= &omap54xx_mpu_hwmod,
@@ -1787,6 +1860,14 @@ static struct omap_hwmod_ocp_if omap54xx_l4_cfg__l3_main_2 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* l3_main_2 -> mmu_ipu */
+static struct omap_hwmod_ocp_if omap54xx_l3_main_2__mmu_ipu = {
+	.master		= &omap54xx_l3_main_2_hwmod,
+	.slave		= &omap54xx_mmu_ipu_hwmod,
+	.clk		= "l3_iclk_div",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 /* l3_main_1 -> l3_main_3 */
 static struct omap_hwmod_ocp_if omap54xx_l3_main_1__l3_main_3 = {
 	.master		= &omap54xx_l3_main_1_hwmod,
@@ -2345,6 +2426,7 @@ static struct omap_hwmod_ocp_if *omap54xx_hwmod_ocp_ifs[] __initdata = {
 	&omap54xx_l4_wkup__counter_32k,
 	&omap54xx_l4_cfg__dma_system,
 	&omap54xx_l4_abe__dmic,
+	&omap54xx_l4_cfg__mmu_dsp,
 	&omap54xx_mpu__emif1,
 	&omap54xx_mpu__emif2,
 	&omap54xx_l4_wkup__gpio1,
@@ -2360,6 +2442,7 @@ static struct omap_hwmod_ocp_if *omap54xx_hwmod_ocp_ifs[] __initdata = {
 	&omap54xx_l4_per__i2c3,
 	&omap54xx_l4_per__i2c4,
 	&omap54xx_l4_per__i2c5,
+	&omap54xx_l3_main_2__mmu_ipu,
 	&omap54xx_l4_wkup__kbd,
 	&omap54xx_l4_cfg__mailbox,
 	&omap54xx_l4_abe__mcbsp1,
-- 
1.8.5.3

[PATCHv2 16/16] ARM: OMAP2+: Remove legacy omap-iommu.c

[Suman Anna]

From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>

With full DT boot, the legacy mode of platform device creation
for OMAP IOMMUs is not needed anymore.

Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
---
 arch/arm/mach-omap2/Makefile     |  3 --
 arch/arm/mach-omap2/omap-iommu.c | 79 ----------------------------------------
 2 files changed, 82 deletions(-)
 delete mode 100644 arch/arm/mach-omap2/omap-iommu.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index e6eec6f..242933f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -226,9 +226,6 @@ obj-$(CONFIG_SOC_DRA7XX)		+= omap_hwmod_7xx_data.o
 obj-$(CONFIG_OMAP3_EMU)			+= emu.o
 obj-$(CONFIG_HW_PERF_EVENTS)		+= pmu.o
 
-iommu-$(CONFIG_OMAP_IOMMU)		:= omap-iommu.o
-obj-y					+= $(iommu-m) $(iommu-y)
-
 ifneq ($(CONFIG_TIDSPBRIDGE),)
 obj-y					+= dsp.o
 endif
diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
deleted file mode 100644
index f1fab56..0000000
--- a/arch/arm/mach-omap2/omap-iommu.c
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * omap iommu: omap device registration
- *
- * Copyright (C) 2008-2009 Nokia Corporation
- *
- * Written by Hiroshi DOYU <Hiroshi.DOYU-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/of.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/err.h>
-#include <linux/slab.h>
-
-#include <linux/platform_data/iommu-omap.h>
-#include "soc.h"
-#include "omap_hwmod.h"
-#include "omap_device.h"
-
-static int __init omap_iommu_dev_init(struct omap_hwmod *oh, void *unused)
-{
-	struct platform_device *pdev;
-	struct iommu_platform_data *pdata;
-	struct omap_mmu_dev_attr *a = (struct omap_mmu_dev_attr *)oh->dev_attr;
-	static int i;
-
-	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return -ENOMEM;
-
-	pdata->name = oh->name;
-	pdata->nr_tlb_entries = a->nr_tlb_entries;
-	pdata->da_start = a->da_start;
-	pdata->da_end = a->da_end;
-
-	if (oh->rst_lines_cnt == 1) {
-		pdata->reset_name = oh->rst_lines->name;
-		pdata->assert_reset = omap_device_assert_hardreset;
-		pdata->deassert_reset = omap_device_deassert_hardreset;
-	}
-
-	pdev = omap_device_build("omap-iommu", i, oh, pdata, sizeof(*pdata));
-
-	kfree(pdata);
-
-	if (IS_ERR(pdev)) {
-		pr_err("%s: device build err: %ld\n", __func__, PTR_ERR(pdev));
-		return PTR_ERR(pdev);
-	}
-
-	i++;
-
-	return 0;
-}
-
-static int __init omap_iommu_init(void)
-{
-	/* If dtb is there, the devices will be created dynamically */
-	if (of_have_populated_dt())
-		return -ENODEV;
-
-	return omap_hwmod_for_each_by_class("mmu", omap_iommu_dev_init, NULL);
-}
-/* must be ready before omap3isp is probed */
-omap_subsys_initcall(omap_iommu_init);
-
-static void __exit omap_iommu_exit(void)
-{
-	/* Do nothing */
-}
-module_exit(omap_iommu_exit);
-
-MODULE_AUTHOR("Hiroshi DOYU");
-MODULE_DESCRIPTION("omap iommu: omap device registration");
-MODULE_LICENSE("GPL v2");
-- 
1.8.5.3

[PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

[Suman Anna]

From: Florian Vaussard <florian.vaussard@epfl.ch>

CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
usage by tidspbridge and other iommu users. The same can be achieved
by marking the DT node disabled, so remove this obsolete flag and
the corresponding hwmod data can be enabled.

Cc: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
[s-anna@ti.com: revise commit log]
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 8 --------
 arch/arm/plat-omap/Kconfig                 | 3 ---
 2 files changed, 11 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 4c3b1e6..81dd071 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -3029,8 +3029,6 @@ static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
 	.flags		= HWMOD_NO_IDLEST,
 };
 
-#ifdef CONFIG_OMAP_IOMMU_IVA2
-
 /* mmu iva */
 
 static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
@@ -3082,8 +3080,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
 	.flags		= HWMOD_NO_IDLEST,
 };
 
-#endif
-
 /* l4_per -> gpio4 */
 static struct omap_hwmod_addr_space omap3xxx_gpio4_addrs[] = {
 	{
@@ -3855,9 +3851,7 @@ static struct omap_hwmod_ocp_if *omap34xx_hwmod_ocp_ifs[] __initdata = {
 	&omap3xxx_l4_core__hdq1w,
 	&omap3xxx_sad2d__l3,
 	&omap3xxx_l4_core__mmu_isp,
-#ifdef CONFIG_OMAP_IOMMU_IVA2
 	&omap3xxx_l3_main__mmu_iva,
-#endif
 	&omap34xx_l4_core__ssi,
 	NULL
 };
@@ -3881,9 +3875,7 @@ static struct omap_hwmod_ocp_if *omap36xx_hwmod_ocp_ifs[] __initdata = {
 	&omap3xxx_l4_core__hdq1w,
 	&omap3xxx_sad2d__l3,
 	&omap3xxx_l4_core__mmu_isp,
-#ifdef CONFIG_OMAP_IOMMU_IVA2
 	&omap3xxx_l3_main__mmu_iva,
-#endif
 	NULL
 };
 
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 436ea97..02fc10d 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -86,9 +86,6 @@ config OMAP_MUX_WARNINGS
 	  to change the pin multiplexing setup.	 When there are no warnings
 	  printed, it's safe to deselect OMAP_MUX for your product.
 
-config OMAP_IOMMU_IVA2
-	bool
-
 config OMAP_MPU_TIMER
 	bool "Use mpu timer"
 	depends on ARCH_OMAP1
-- 
1.8.5.3

Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

[Laurent Pinchart]

On Thursday 13 February 2014 12:15:39 Suman Anna wrote:
> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> 
> CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
> usage by tidspbridge and other iommu users. The same can be achieved
> by marking the DT node disabled, so remove this obsolete flag and
> the corresponding hwmod data can be enabled.
> 
> Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> [s-anna-l0cyMroinI0@public.gmane.org: revise commit log]
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

> ---
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 8 --------
>  arch/arm/plat-omap/Kconfig                 | 3 ---
>  2 files changed, 11 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c index 4c3b1e6..81dd071 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -3029,8 +3029,6 @@ static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
>  	.flags		= HWMOD_NO_IDLEST,
>  };
> 
> -#ifdef CONFIG_OMAP_IOMMU_IVA2
> -
>  /* mmu iva */
> 
>  static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
> @@ -3082,8 +3080,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>  	.flags		= HWMOD_NO_IDLEST,
>  };
> 
> -#endif
> -
>  /* l4_per -> gpio4 */
>  static struct omap_hwmod_addr_space omap3xxx_gpio4_addrs[] = {
>  	{
> @@ -3855,9 +3851,7 @@ static struct omap_hwmod_ocp_if
> *omap34xx_hwmod_ocp_ifs[] __initdata = { &omap3xxx_l4_core__hdq1w,
>  	&omap3xxx_sad2d__l3,
>  	&omap3xxx_l4_core__mmu_isp,
> -#ifdef CONFIG_OMAP_IOMMU_IVA2
>  	&omap3xxx_l3_main__mmu_iva,
> -#endif
>  	&omap34xx_l4_core__ssi,
>  	NULL
>  };
> @@ -3881,9 +3875,7 @@ static struct omap_hwmod_ocp_if
> *omap36xx_hwmod_ocp_ifs[] __initdata = { &omap3xxx_l4_core__hdq1w,
>  	&omap3xxx_sad2d__l3,
>  	&omap3xxx_l4_core__mmu_isp,
> -#ifdef CONFIG_OMAP_IOMMU_IVA2
>  	&omap3xxx_l3_main__mmu_iva,
> -#endif
>  	NULL
>  };
> 
> diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> index 436ea97..02fc10d 100644
> --- a/arch/arm/plat-omap/Kconfig
> +++ b/arch/arm/plat-omap/Kconfig
> @@ -86,9 +86,6 @@ config OMAP_MUX_WARNINGS
>  	  to change the pin multiplexing setup.	 When there are no warnings
>  	  printed, it's safe to deselect OMAP_MUX for your product.
> 
> -config OMAP_IOMMU_IVA2
> -	bool
> -
>  config OMAP_MPU_TIMER
>  	bool "Use mpu timer"
>  	depends on ARCH_OMAP1

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140225 13:18]:
> On Thursday 13 February 2014 12:15:39 Suman Anna wrote:
> > From: Florian Vaussard <florian.vaussard@epfl.ch>
> > 
> > CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
> > usage by tidspbridge and other iommu users. The same can be achieved
> > by marking the DT node disabled, so remove this obsolete flag and
> > the corresponding hwmod data can be enabled.
> > 
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> > [s-anna@ti.com: revise commit log]
> > Signed-off-by: Suman Anna <s-anna@ti.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140225 13:18]:
> On Thursday 13 February 2014 12:15:39 Suman Anna wrote:
> > From: Florian Vaussard <florian.vaussard@epfl.ch>
> > 
> > CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
> > usage by tidspbridge and other iommu users. The same can be achieved
> > by marking the DT node disabled, so remove this obsolete flag and
> > the corresponding hwmod data can be enabled.
> > 
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> > [s-anna@ti.com: revise commit log]
> > Signed-off-by: Suman Anna <s-anna@ti.com>
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

[Paul Walmsley]

On Thu, 13 Feb 2014, Suman Anna wrote:

> From: Florian Vaussard <florian.vaussard@epfl.ch>
> 
> CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
> usage by tidspbridge and other iommu users. The same can be achieved
> by marking the DT node disabled, so remove this obsolete flag and
> the corresponding hwmod data can be enabled.
> 
> Cc: Paul Walmsley <paul@pwsan.com>
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> [s-anna@ti.com: revise commit log]
> Signed-off-by: Suman Anna <s-anna@ti.com>

Acked-by: Paul Walmsley <paul@pwsan.com>

Am assuming that someone else will be merging this as part of the entire 
series.

- Paul

Re: [PATCHv2 08/16] ARM: OMAP3: remove deprecated CONFIG_OMAP_IOMMU_IVA2

[Suman Anna]

Paul,

On 02/28/2014 01:58 PM, Paul Walmsley wrote:
> On Thu, 13 Feb 2014, Suman Anna wrote:
>
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> CONFIG_OMAP_IOMMU_IVA2 was defined originally to avoid conflicting
>> usage by tidspbridge and other iommu users. The same can be achieved
>> by marking the DT node disabled, so remove this obsolete flag and
>> the corresponding hwmod data can be enabled.
>>
>> Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>> [s-anna-l0cyMroinI0@public.gmane.org: revise commit log]
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>
> Acked-by: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
>
> Am assuming that someone else will be merging this as part of the entire
> series.
>

Thanks, I am expecting Tony to pick up all the arch/arm/mach-omap2/ 
patches. I am just about to send the v3 including all the acks received 
so far.

regards
Suman

[PATCHv2 13/16] ARM: OMAP2+: extend iommu pdata-quirks to OMAP5

[Suman Anna]

OMAP5 has the same iommus as OMAP4, so extend the OMAP4
iommu pdata quirks for OMAP5 as well.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/pdata-quirks.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index 74e094a..551877f 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -193,7 +193,9 @@ static void __init omap4_panda_legacy_init(void)
 	legacy_init_ehci_clk("auxclk3_ck");
 	legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
 }
+#endif
 
+#if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
 static struct iommu_platform_data omap4_iommu_pdata = {
 	.reset_name = "mmu_cache",
 	.assert_reset = omap_device_assert_hardreset,
@@ -264,6 +266,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
 #ifdef CONFIG_ARCH_OMAP4
 	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
 	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
+#endif
+#if defined(CONFIG_ARCH_OMAP4) || defined(CONFIG_SOC_OMAP5)
 	OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
 		       &omap4_iommu_pdata),
 	OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
-- 
1.8.5.3

[PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

[Suman Anna]

From: Florian Vaussard <florian.vaussard@epfl.ch>

The irq numbers, ocp address space and device attribute data
have all been cleaned up for OMAP3 IOMMUs. All this data is
populated via the corresponding dt node.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 46 ------------------------------
 1 file changed, 46 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 9c7e23a..d68c131 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -24,7 +24,6 @@
 #include "l4_3xxx.h"
 #include <linux/platform_data/asoc-ti-mcbsp.h>
 #include <linux/platform_data/spi-omap2-mcspi.h>
-#include <linux/platform_data/iommu-omap.h>
 #include <linux/platform_data/mailbox-omap.h>
 #include <plat/dmtimer.h>
 
@@ -2991,83 +2990,39 @@ static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
 
 /* mmu isp */
 
-static struct omap_mmu_dev_attr mmu_isp_dev_attr = {
-	.da_start	= 0x0,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 8,
-};
-
 static struct omap_hwmod omap3xxx_mmu_isp_hwmod;
-static struct omap_hwmod_irq_info omap3xxx_mmu_isp_irqs[] = {
-	{ .irq = 24 + OMAP_INTC_START, },
-	{ .irq = -1 }
-};
-
-static struct omap_hwmod_addr_space omap3xxx_mmu_isp_addrs[] = {
-	{
-		.pa_start	= 0x480bd400,
-		.pa_end		= 0x480bd47f,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
 
 /* l4_core -> mmu isp */
 static struct omap_hwmod_ocp_if omap3xxx_l4_core__mmu_isp = {
 	.master		= &omap3xxx_l4_core_hwmod,
 	.slave		= &omap3xxx_mmu_isp_hwmod,
-	.addr		= omap3xxx_mmu_isp_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
 static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
 	.name		= "mmu_isp",
 	.class		= &omap3xxx_mmu_hwmod_class,
-	.mpu_irqs	= omap3xxx_mmu_isp_irqs,
 	.main_clk	= "cam_ick",
-	.dev_attr	= &mmu_isp_dev_attr,
 	.flags		= HWMOD_NO_IDLEST,
 };
 
 /* mmu iva */
 
-static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
-	.da_start	= 0x11000000,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 32,
-};
-
 static struct omap_hwmod omap3xxx_mmu_iva_hwmod;
-static struct omap_hwmod_irq_info omap3xxx_mmu_iva_irqs[] = {
-	{ .irq = 28 + OMAP_INTC_START, },
-	{ .irq = -1 }
-};
-
 static struct omap_hwmod_rst_info omap3xxx_mmu_iva_resets[] = {
 	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
 };
 
-static struct omap_hwmod_addr_space omap3xxx_mmu_iva_addrs[] = {
-	{
-		.pa_start	= 0x5d000000,
-		.pa_end		= 0x5d00007f,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
-
 /* l3_main -> iva mmu */
 static struct omap_hwmod_ocp_if omap3xxx_l3_main__mmu_iva = {
 	.master		= &omap3xxx_l3_main_hwmod,
 	.slave		= &omap3xxx_mmu_iva_hwmod,
-	.addr		= omap3xxx_mmu_iva_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
 static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
 	.name		= "mmu_iva",
 	.class		= &omap3xxx_mmu_hwmod_class,
-	.mpu_irqs	= omap3xxx_mmu_iva_irqs,
 	.clkdm_name	= "iva2_clkdm",
 	.rst_lines	= omap3xxx_mmu_iva_resets,
 	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_mmu_iva_resets),
@@ -3080,7 +3035,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
 			.idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
 		},
 	},
-	.dev_attr	= &mmu_iva_dev_attr,
 	.flags		= HWMOD_NO_IDLEST,
 };
 
-- 
1.8.5.3

Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

[Tony Lindgren]

* Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> 
> The irq numbers, ocp address space and device attribute data
> have all been cleaned up for OMAP3 IOMMUs. All this data is
> populated via the corresponding dt node.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>

This will need to wait until we've made omap3 to be DT only
as this will break idling of things for the legacy booting.

Regards,

Tony

> ---
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 46 ------------------------------
>  1 file changed, 46 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index 9c7e23a..d68c131 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -24,7 +24,6 @@
>  #include "l4_3xxx.h"
>  #include <linux/platform_data/asoc-ti-mcbsp.h>
>  #include <linux/platform_data/spi-omap2-mcspi.h>
> -#include <linux/platform_data/iommu-omap.h>
>  #include <linux/platform_data/mailbox-omap.h>
>  #include <plat/dmtimer.h>
>  
> @@ -2991,83 +2990,39 @@ static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
>  
>  /* mmu isp */
>  
> -static struct omap_mmu_dev_attr mmu_isp_dev_attr = {
> -	.da_start	= 0x0,
> -	.da_end		= 0xfffff000,
> -	.nr_tlb_entries = 8,
> -};
> -
>  static struct omap_hwmod omap3xxx_mmu_isp_hwmod;
> -static struct omap_hwmod_irq_info omap3xxx_mmu_isp_irqs[] = {
> -	{ .irq = 24 + OMAP_INTC_START, },
> -	{ .irq = -1 }
> -};
> -
> -static struct omap_hwmod_addr_space omap3xxx_mmu_isp_addrs[] = {
> -	{
> -		.pa_start	= 0x480bd400,
> -		.pa_end		= 0x480bd47f,
> -		.flags		= ADDR_TYPE_RT,
> -	},
> -	{ }
> -};
>  
>  /* l4_core -> mmu isp */
>  static struct omap_hwmod_ocp_if omap3xxx_l4_core__mmu_isp = {
>  	.master		= &omap3xxx_l4_core_hwmod,
>  	.slave		= &omap3xxx_mmu_isp_hwmod,
> -	.addr		= omap3xxx_mmu_isp_addrs,
>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
>  static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
>  	.name		= "mmu_isp",
>  	.class		= &omap3xxx_mmu_hwmod_class,
> -	.mpu_irqs	= omap3xxx_mmu_isp_irqs,
>  	.main_clk	= "cam_ick",
> -	.dev_attr	= &mmu_isp_dev_attr,
>  	.flags		= HWMOD_NO_IDLEST,
>  };
>  
>  /* mmu iva */
>  
> -static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
> -	.da_start	= 0x11000000,
> -	.da_end		= 0xfffff000,
> -	.nr_tlb_entries = 32,
> -};
> -
>  static struct omap_hwmod omap3xxx_mmu_iva_hwmod;
> -static struct omap_hwmod_irq_info omap3xxx_mmu_iva_irqs[] = {
> -	{ .irq = 28 + OMAP_INTC_START, },
> -	{ .irq = -1 }
> -};
> -
>  static struct omap_hwmod_rst_info omap3xxx_mmu_iva_resets[] = {
>  	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
>  };
>  
> -static struct omap_hwmod_addr_space omap3xxx_mmu_iva_addrs[] = {
> -	{
> -		.pa_start	= 0x5d000000,
> -		.pa_end		= 0x5d00007f,
> -		.flags		= ADDR_TYPE_RT,
> -	},
> -	{ }
> -};
> -
>  /* l3_main -> iva mmu */
>  static struct omap_hwmod_ocp_if omap3xxx_l3_main__mmu_iva = {
>  	.master		= &omap3xxx_l3_main_hwmod,
>  	.slave		= &omap3xxx_mmu_iva_hwmod,
> -	.addr		= omap3xxx_mmu_iva_addrs,
>  	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>  };
>  
>  static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>  	.name		= "mmu_iva",
>  	.class		= &omap3xxx_mmu_hwmod_class,
> -	.mpu_irqs	= omap3xxx_mmu_iva_irqs,
>  	.clkdm_name	= "iva2_clkdm",
>  	.rst_lines	= omap3xxx_mmu_iva_resets,
>  	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_mmu_iva_resets),
> @@ -3080,7 +3035,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>  			.idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
>  		},
>  	},
> -	.dev_attr	= &mmu_iva_dev_attr,
>  	.flags		= HWMOD_NO_IDLEST,
>  };
>  
> -- 
> 1.8.5.3
> 

Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

[Suman Anna]

Tony,

On 02/26/2014 11:18 AM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> The irq numbers, ocp address space and device attribute data
>> have all been cleaned up for OMAP3 IOMMUs. All this data is
>> populated via the corresponding dt node.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>
> This will need to wait until we've made omap3 to be DT only
> as this will break idling of things for the legacy booting.
>

OK, will drop this and I will adjust Patch 9 to support the legacy boot 
for OMAP3 ISP.

regards
Suman

>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 46 ------------------------------
>>   1 file changed, 46 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> index 9c7e23a..d68c131 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> @@ -24,7 +24,6 @@
>>   #include "l4_3xxx.h"
>>   #include <linux/platform_data/asoc-ti-mcbsp.h>
>>   #include <linux/platform_data/spi-omap2-mcspi.h>
>> -#include <linux/platform_data/iommu-omap.h>
>>   #include <linux/platform_data/mailbox-omap.h>
>>   #include <plat/dmtimer.h>
>>
>> @@ -2991,83 +2990,39 @@ static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
>>
>>   /* mmu isp */
>>
>> -static struct omap_mmu_dev_attr mmu_isp_dev_attr = {
>> -	.da_start	= 0x0,
>> -	.da_end		= 0xfffff000,
>> -	.nr_tlb_entries = 8,
>> -};
>> -
>>   static struct omap_hwmod omap3xxx_mmu_isp_hwmod;
>> -static struct omap_hwmod_irq_info omap3xxx_mmu_isp_irqs[] = {
>> -	{ .irq = 24 + OMAP_INTC_START, },
>> -	{ .irq = -1 }
>> -};
>> -
>> -static struct omap_hwmod_addr_space omap3xxx_mmu_isp_addrs[] = {
>> -	{
>> -		.pa_start	= 0x480bd400,
>> -		.pa_end		= 0x480bd47f,
>> -		.flags		= ADDR_TYPE_RT,
>> -	},
>> -	{ }
>> -};
>>
>>   /* l4_core -> mmu isp */
>>   static struct omap_hwmod_ocp_if omap3xxx_l4_core__mmu_isp = {
>>   	.master		= &omap3xxx_l4_core_hwmod,
>>   	.slave		= &omap3xxx_mmu_isp_hwmod,
>> -	.addr		= omap3xxx_mmu_isp_addrs,
>>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>   };
>>
>>   static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
>>   	.name		= "mmu_isp",
>>   	.class		= &omap3xxx_mmu_hwmod_class,
>> -	.mpu_irqs	= omap3xxx_mmu_isp_irqs,
>>   	.main_clk	= "cam_ick",
>> -	.dev_attr	= &mmu_isp_dev_attr,
>>   	.flags		= HWMOD_NO_IDLEST,
>>   };
>>
>>   /* mmu iva */
>>
>> -static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
>> -	.da_start	= 0x11000000,
>> -	.da_end		= 0xfffff000,
>> -	.nr_tlb_entries = 32,
>> -};
>> -
>>   static struct omap_hwmod omap3xxx_mmu_iva_hwmod;
>> -static struct omap_hwmod_irq_info omap3xxx_mmu_iva_irqs[] = {
>> -	{ .irq = 28 + OMAP_INTC_START, },
>> -	{ .irq = -1 }
>> -};
>> -
>>   static struct omap_hwmod_rst_info omap3xxx_mmu_iva_resets[] = {
>>   	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
>>   };
>>
>> -static struct omap_hwmod_addr_space omap3xxx_mmu_iva_addrs[] = {
>> -	{
>> -		.pa_start	= 0x5d000000,
>> -		.pa_end		= 0x5d00007f,
>> -		.flags		= ADDR_TYPE_RT,
>> -	},
>> -	{ }
>> -};
>> -
>>   /* l3_main -> iva mmu */
>>   static struct omap_hwmod_ocp_if omap3xxx_l3_main__mmu_iva = {
>>   	.master		= &omap3xxx_l3_main_hwmod,
>>   	.slave		= &omap3xxx_mmu_iva_hwmod,
>> -	.addr		= omap3xxx_mmu_iva_addrs,
>>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>   };
>>
>>   static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>>   	.name		= "mmu_iva",
>>   	.class		= &omap3xxx_mmu_hwmod_class,
>> -	.mpu_irqs	= omap3xxx_mmu_iva_irqs,
>>   	.clkdm_name	= "iva2_clkdm",
>>   	.rst_lines	= omap3xxx_mmu_iva_resets,
>>   	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_mmu_iva_resets),
>> @@ -3080,7 +3035,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>>   			.idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
>>   		},
>>   	},
>> -	.dev_attr	= &mmu_iva_dev_attr,
>>   	.flags		= HWMOD_NO_IDLEST,
>>   };
>>
>> --
>> 1.8.5.3
>>

Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

[Florian Vaussard]

Hi,

On 02/26/2014 06:59 PM, Suman Anna wrote:
> Tony,
> 
> On 02/26/2014 11:18 AM, Tony Lindgren wrote:
>> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>
>>> The irq numbers, ocp address space and device attribute data
>>> have all been cleaned up for OMAP3 IOMMUs. All this data is
>>> populated via the corresponding dt node.
>>>
>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>
>> This will need to wait until we've made omap3 to be DT only
>> as this will break idling of things for the legacy booting.
>>
> 
> OK, will drop this and I will adjust Patch 9 to support the legacy boot
> for OMAP3 ISP.
> 

BTW, Tony do you have a new window for omap3 to be DT onyl?

Regards,
Florian

Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs

[Tony Lindgren]

* Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org> [140227 01:19]:
> Hi,
> 
> On 02/26/2014 06:59 PM, Suman Anna wrote:
> > Tony,
> > 
> > On 02/26/2014 11:18 AM, Tony Lindgren wrote:
> >> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
> >>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >>>
> >>> The irq numbers, ocp address space and device attribute data
> >>> have all been cleaned up for OMAP3 IOMMUs. All this data is
> >>> populated via the corresponding dt node.
> >>>
> >>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> >>
> >> This will need to wait until we've made omap3 to be DT only
> >> as this will break idling of things for the legacy booting.
> >>
> > 
> > OK, will drop this and I will adjust Patch 9 to support the legacy boot
> > for OMAP3 ISP.
> > 
> 
> BTW, Tony do you have a new window for omap3 to be DT onyl?

I think a good point would be when we have DSS working with DT,
and have a .dts file for the Pandora board.

Regards,

Tony

[PATCHv2 15/16] ARM: OMAP4: hwmod data: cleanup data for IOMMUs

[Suman Anna]

From: Florian Vaussard <florian.vaussard@epfl.ch>

The device attribute data and ocp address space have all been
cleaned up for OMAP4 iommus. All this data is populated via
the corresponding dt node.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 35 ------------------------------
 1 file changed, 35 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 3318cae9..caca6c2 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -29,7 +29,6 @@
 
 #include <linux/platform_data/spi-omap2-mcspi.h>
 #include <linux/platform_data/asoc-ti-mcbsp.h>
-#include <linux/platform_data/iommu-omap.h>
 #include <plat/dmtimer.h>
 
 #include "omap_hwmod.h"
@@ -2083,32 +2082,16 @@ static struct omap_hwmod_class omap44xx_mmu_hwmod_class = {
 
 /* mmu ipu */
 
-static struct omap_mmu_dev_attr mmu_ipu_dev_attr = {
-	.da_start	= 0x0,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 32,
-};
-
 static struct omap_hwmod omap44xx_mmu_ipu_hwmod;
 static struct omap_hwmod_rst_info omap44xx_mmu_ipu_resets[] = {
 	{ .name = "mmu_cache", .rst_shift = 2 },
 };
 
-static struct omap_hwmod_addr_space omap44xx_mmu_ipu_addrs[] = {
-	{
-		.pa_start	= 0x55082000,
-		.pa_end		= 0x550820ff,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
-
 /* l3_main_2 -> mmu_ipu */
 static struct omap_hwmod_ocp_if omap44xx_l3_main_2__mmu_ipu = {
 	.master		= &omap44xx_l3_main_2_hwmod,
 	.slave		= &omap44xx_mmu_ipu_hwmod,
 	.clk		= "l3_div_ck",
-	.addr		= omap44xx_mmu_ipu_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
@@ -2127,37 +2110,20 @@ static struct omap_hwmod omap44xx_mmu_ipu_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.dev_attr	= &mmu_ipu_dev_attr,
 };
 
 /* mmu dsp */
 
-static struct omap_mmu_dev_attr mmu_dsp_dev_attr = {
-	.da_start	= 0x0,
-	.da_end		= 0xfffff000,
-	.nr_tlb_entries = 32,
-};
-
 static struct omap_hwmod omap44xx_mmu_dsp_hwmod;
 static struct omap_hwmod_rst_info omap44xx_mmu_dsp_resets[] = {
 	{ .name = "mmu_cache", .rst_shift = 1 },
 };
 
-static struct omap_hwmod_addr_space omap44xx_mmu_dsp_addrs[] = {
-	{
-		.pa_start	= 0x4a066000,
-		.pa_end		= 0x4a0660ff,
-		.flags		= ADDR_TYPE_RT,
-	},
-	{ }
-};
-
 /* l4_cfg -> dsp */
 static struct omap_hwmod_ocp_if omap44xx_l4_cfg__mmu_dsp = {
 	.master		= &omap44xx_l4_cfg_hwmod,
 	.slave		= &omap44xx_mmu_dsp_hwmod,
 	.clk		= "l4_div_ck",
-	.addr		= omap44xx_mmu_dsp_addrs,
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
@@ -2176,7 +2142,6 @@ static struct omap_hwmod omap44xx_mmu_dsp_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
-	.dev_attr	= &mmu_dsp_dev_attr,
 };
 
 /*
-- 
1.8.5.3