[PATCH v11 13/27] iommu/exynos: support for device tree

[PATCH v11 13/27] iommu/exynos: support for device tree

[Cho KyongHo]

This commit adds device tree support for System MMU.

Signed-off-by: Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
 drivers/iommu/Kconfig        |    5 ++---
 drivers/iommu/exynos-iommu.c |   21 +++++++++++++++++----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index df56e4c..22af807 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -178,16 +178,15 @@ config TEGRA_IOMMU_SMMU
 
 config EXYNOS_IOMMU
 	bool "Exynos IOMMU Support"
-	depends on ARCH_EXYNOS && EXYNOS_DEV_SYSMMU
+	depends on ARCH_EXYNOS
 	select IOMMU_API
+	default n
 	help
 	  Support for the IOMMU(System MMU) of Samsung Exynos application
 	  processor family. This enables H/W multimedia accellerators to see
 	  non-linear physical memory chunks as a linear memory in their
 	  address spaces
 
-	  If unsure, say N here.
-
 config EXYNOS_IOMMU_DEBUG
 	bool "Debugging log for Exynos IOMMU"
 	depends on EXYNOS_IOMMU
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 33b424d..34feb04 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -26,6 +26,7 @@
 #include <linux/list.h>
 #include <linux/memblock.h>
 #include <linux/export.h>
+#include <linux/of.h>
 
 #include <asm/cacheflush.h>
 #include <asm/pgtable.h>
@@ -497,7 +498,7 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
 	read_unlock_irqrestore(&data->lock, flags);
 }
 
-static int exynos_sysmmu_probe(struct platform_device *pdev)
+static int __init exynos_sysmmu_probe(struct platform_device *pdev)
 {
 	int irq, ret;
 	struct device *dev = &pdev->dev;
@@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static struct platform_driver exynos_sysmmu_driver = {
-	.probe		= exynos_sysmmu_probe,
-	.driver		= {
+#ifdef CONFIG_OF
+static struct of_device_id sysmmu_of_match[] __initconst = {
+	{ .compatible	= "samsung,sysmmu-v1", },
+	{ .compatible	= "samsung,sysmmu-v2", },
+	{ .compatible	= "samsung,sysmmu-v3.1", },
+	{ .compatible	= "samsung,sysmmu-v3.2", },
+	{ .compatible	= "samsung,sysmmu-v3.3", },
+	{ },
+};
+#endif
+
+static struct platform_driver exynos_sysmmu_driver __refdata = {
+	.probe	= exynos_sysmmu_probe,
+	.driver	= {
 		.owner		= THIS_MODULE,
 		.name		= "exynos-sysmmu",
+		.of_match_table	= of_match_ptr(sysmmu_of_match),
 	}
 };
 
-- 
1.7.9.5

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Tomasz Figa]

Hi KyongHo,

On 14.03.2014 06:09, Cho KyongHo wrote:
> This commit adds device tree support for System MMU.
>
> Signed-off-by: Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>   drivers/iommu/Kconfig        |    5 ++---
>   drivers/iommu/exynos-iommu.c |   21 +++++++++++++++++----
>   2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index df56e4c..22af807 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -178,16 +178,15 @@ config TEGRA_IOMMU_SMMU
>
>   config EXYNOS_IOMMU
>   	bool "Exynos IOMMU Support"
> -	depends on ARCH_EXYNOS && EXYNOS_DEV_SYSMMU
> +	depends on ARCH_EXYNOS
>   	select IOMMU_API
> +	default n
>   	help
>   	  Support for the IOMMU(System MMU) of Samsung Exynos application

nit: There should be a white space before the opening parenthesis.

>   	  processor family. This enables H/W multimedia accellerators to see

typo: s/accellerators/accelerators/

>   	  non-linear physical memory chunks as a linear memory in their
>   	  address spaces
>
> -	  If unsure, say N here.
> -
>   config EXYNOS_IOMMU_DEBUG
>   	bool "Debugging log for Exynos IOMMU"
>   	depends on EXYNOS_IOMMU
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33b424d..34feb04 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -26,6 +26,7 @@
>   #include <linux/list.h>
>   #include <linux/memblock.h>
>   #include <linux/export.h>
> +#include <linux/of.h>
>
>   #include <asm/cacheflush.h>
>   #include <asm/pgtable.h>
> @@ -497,7 +498,7 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
>   	read_unlock_irqrestore(&data->lock, flags);
>   }
>
> -static int exynos_sysmmu_probe(struct platform_device *pdev)
> +static int __init exynos_sysmmu_probe(struct platform_device *pdev)
>   {
>   	int irq, ret;
>   	struct device *dev = &pdev->dev;
> @@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
>   	return 0;
>   }
>
> -static struct platform_driver exynos_sysmmu_driver = {
> -	.probe		= exynos_sysmmu_probe,
> -	.driver		= {
> +#ifdef CONFIG_OF
> +static struct of_device_id sysmmu_of_match[] __initconst = {
> +	{ .compatible	= "samsung,sysmmu-v1", },
> +	{ .compatible	= "samsung,sysmmu-v2", },
> +	{ .compatible	= "samsung,sysmmu-v3.1", },
> +	{ .compatible	= "samsung,sysmmu-v3.2", },
> +	{ .compatible	= "samsung,sysmmu-v3.3", },

Do you need all these compatible strings? I mean, are there any 
implementation differences that can't be identified by reading IP 
registers, such as REG_MMU_VERSION?

Best regards,
Tomasz

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Cho KyongHo]

On Fri, 14 Mar 2014 14:39:33 +0100, Tomasz Figa wrote:
> Hi KyongHo,
> 
> On 14.03.2014 06:09, Cho KyongHo wrote:
> > This commit adds device tree support for System MMU.
> >
> > Signed-off-by: Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >   drivers/iommu/Kconfig        |    5 ++---
> >   drivers/iommu/exynos-iommu.c |   21 +++++++++++++++++----
> >   2 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index df56e4c..22af807 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -178,16 +178,15 @@ config TEGRA_IOMMU_SMMU
> >
> >   config EXYNOS_IOMMU
> >   	bool "Exynos IOMMU Support"
> > -	depends on ARCH_EXYNOS && EXYNOS_DEV_SYSMMU
> > +	depends on ARCH_EXYNOS
> >   	select IOMMU_API
> > +	default n
> >   	help
> >   	  Support for the IOMMU(System MMU) of Samsung Exynos application
> 
> nit: There should be a white space before the opening parenthesis.
> 
Ok. :)

> >   	  processor family. This enables H/W multimedia accellerators to see
> 
> typo: s/accellerators/accelerators/
> 

Ok.

> >   	  non-linear physical memory chunks as a linear memory in their
> >   	  address spaces
> >
> > -	  If unsure, say N here.
> > -
> >   config EXYNOS_IOMMU_DEBUG
> >   	bool "Debugging log for Exynos IOMMU"
> >   	depends on EXYNOS_IOMMU
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 33b424d..34feb04 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -26,6 +26,7 @@
> >   #include <linux/list.h>
> >   #include <linux/memblock.h>
> >   #include <linux/export.h>
> > +#include <linux/of.h>
> >
> >   #include <asm/cacheflush.h>
> >   #include <asm/pgtable.h>
> > @@ -497,7 +498,7 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
> >   	read_unlock_irqrestore(&data->lock, flags);
> >   }
> >
> > -static int exynos_sysmmu_probe(struct platform_device *pdev)
> > +static int __init exynos_sysmmu_probe(struct platform_device *pdev)
> >   {
> >   	int irq, ret;
> >   	struct device *dev = &pdev->dev;
> > @@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> >   	return 0;
> >   }
> >
> > -static struct platform_driver exynos_sysmmu_driver = {
> > -	.probe		= exynos_sysmmu_probe,
> > -	.driver		= {
> > +#ifdef CONFIG_OF
> > +static struct of_device_id sysmmu_of_match[] __initconst = {
> > +	{ .compatible	= "samsung,sysmmu-v1", },
> > +	{ .compatible	= "samsung,sysmmu-v2", },
> > +	{ .compatible	= "samsung,sysmmu-v3.1", },
> > +	{ .compatible	= "samsung,sysmmu-v3.2", },
> > +	{ .compatible	= "samsung,sysmmu-v3.3", },
> 
> Do you need all these compatible strings? I mean, are there any 
> implementation differences that can't be identified by reading IP 
> registers, such as REG_MMU_VERSION?
> 

Unfortunately, there is a SoC which overrides REG_MMU_VERSION with
a value for RTL designers and it is not related to System MMU
versions.

Thank you.

KyongHo

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Tomasz Figa]

On 18.03.2014 11:52, Cho KyongHo wrote:
> On Fri, 14 Mar 2014 14:39:33 +0100, Tomasz Figa wrote:
>>> @@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
>>>    	return 0;
>>>    }
>>>
>>> -static struct platform_driver exynos_sysmmu_driver = {
>>> -	.probe		= exynos_sysmmu_probe,
>>> -	.driver		= {
>>> +#ifdef CONFIG_OF
>>> +static struct of_device_id sysmmu_of_match[] __initconst = {
>>> +	{ .compatible	= "samsung,sysmmu-v1", },
>>> +	{ .compatible	= "samsung,sysmmu-v2", },
>>> +	{ .compatible	= "samsung,sysmmu-v3.1", },
>>> +	{ .compatible	= "samsung,sysmmu-v3.2", },
>>> +	{ .compatible	= "samsung,sysmmu-v3.3", },
>>
>> Do you need all these compatible strings? I mean, are there any
>> implementation differences that can't be identified by reading IP
>> registers, such as REG_MMU_VERSION?
>>
>
> Unfortunately, there is a SoC which overrides REG_MMU_VERSION with
> a value for RTL designers and it is not related to System MMU
> versions.

OK.

What about having a generic compatible string for Samsung SysMMU then, 
but an additional property that can override the version to account for 
such brokenness? If not provided, the version would be read from 
REG_MMU_VERSION.

Best regards,
Tomasz

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Cho KyongHo]

On Tue, 18 Mar 2014 16:25:11 +0100, Tomasz Figa wrote:
> On 18.03.2014 11:52, Cho KyongHo wrote:
> > On Fri, 14 Mar 2014 14:39:33 +0100, Tomasz Figa wrote:
> >>> @@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> >>>    	return 0;
> >>>    }
> >>>
> >>> -static struct platform_driver exynos_sysmmu_driver = {
> >>> -	.probe		= exynos_sysmmu_probe,
> >>> -	.driver		= {
> >>> +#ifdef CONFIG_OF
> >>> +static struct of_device_id sysmmu_of_match[] __initconst = {
> >>> +	{ .compatible	= "samsung,sysmmu-v1", },
> >>> +	{ .compatible	= "samsung,sysmmu-v2", },
> >>> +	{ .compatible	= "samsung,sysmmu-v3.1", },
> >>> +	{ .compatible	= "samsung,sysmmu-v3.2", },
> >>> +	{ .compatible	= "samsung,sysmmu-v3.3", },
> >>
> >> Do you need all these compatible strings? I mean, are there any
> >> implementation differences that can't be identified by reading IP
> >> registers, such as REG_MMU_VERSION?
> >>
> >
> > Unfortunately, there is a SoC which overrides REG_MMU_VERSION with
> > a value for RTL designers and it is not related to System MMU
> > versions.
> 
> OK.
> 
> What about having a generic compatible string for Samsung SysMMU then, 
> but an additional property that can override the version to account for 
> such brokenness? If not provided, the version would be read from 
> REG_MMU_VERSION.
> 

Yes it is one of possible idea.
Let me think what better way is.

Thank you.

KyongHo

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Sachin Kamat]

Hi KyongHo,

On 14 March 2014 10:39, Cho KyongHo <pullip.cho@samsung.com> wrote:
> This commit adds device tree support for System MMU.
>
> Signed-off-by: Cho KyongHo <pullip.cho@samsung.com>
> ---
>  drivers/iommu/Kconfig        |    5 ++---
>  drivers/iommu/exynos-iommu.c |   21 +++++++++++++++++----
>  2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index df56e4c..22af807 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -178,16 +178,15 @@ config TEGRA_IOMMU_SMMU
>
>  config EXYNOS_IOMMU
>         bool "Exynos IOMMU Support"
> -       depends on ARCH_EXYNOS && EXYNOS_DEV_SYSMMU
> +       depends on ARCH_EXYNOS
>         select IOMMU_API
> +       default n

This is not needed as it is the default choice.

>         help
>           Support for the IOMMU(System MMU) of Samsung Exynos application
>           processor family. This enables H/W multimedia accellerators to see
>           non-linear physical memory chunks as a linear memory in their
>           address spaces
>
> -         If unsure, say N here.
> -
>  config EXYNOS_IOMMU_DEBUG
>         bool "Debugging log for Exynos IOMMU"
>         depends on EXYNOS_IOMMU
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 33b424d..34feb04 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -26,6 +26,7 @@
>  #include <linux/list.h>
>  #include <linux/memblock.h>
>  #include <linux/export.h>
> +#include <linux/of.h>
>
>  #include <asm/cacheflush.h>
>  #include <asm/pgtable.h>
> @@ -497,7 +498,7 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
>         read_unlock_irqrestore(&data->lock, flags);
>  }
>
> -static int exynos_sysmmu_probe(struct platform_device *pdev)
> +static int __init exynos_sysmmu_probe(struct platform_device *pdev)
>  {
>         int irq, ret;
>         struct device *dev = &pdev->dev;
> @@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
>         return 0;
>  }
>
> -static struct platform_driver exynos_sysmmu_driver = {
> -       .probe          = exynos_sysmmu_probe,
> -       .driver         = {
> +#ifdef CONFIG_OF

This is not needed as Exynos is DT only platform.

> +static struct of_device_id sysmmu_of_match[] __initconst = {
> +       { .compatible   = "samsung,sysmmu-v1", },
> +       { .compatible   = "samsung,sysmmu-v2", },
> +       { .compatible   = "samsung,sysmmu-v3.1", },
> +       { .compatible   = "samsung,sysmmu-v3.2", },
> +       { .compatible   = "samsung,sysmmu-v3.3", },
> +       { },
> +};
> +#endif
> +
> +static struct platform_driver exynos_sysmmu_driver __refdata = {
> +       .probe  = exynos_sysmmu_probe,
> +       .driver = {
>                 .owner          = THIS_MODULE,
>                 .name           = "exynos-sysmmu",
> +               .of_match_table = of_match_ptr(sysmmu_of_match),

of_match_ptr is not needed for the same reason as above.


-- 
With warm regards,
Sachin

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Cho KyongHo]

On Fri, 14 Mar 2014 20:57:42 +0530, Sachin Kamat wrote:
> Hi KyongHo,
> 
> On 14 March 2014 10:39, Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > This commit adds device tree support for System MMU.
> >
> > Signed-off-by: Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  drivers/iommu/Kconfig        |    5 ++---
> >  drivers/iommu/exynos-iommu.c |   21 +++++++++++++++++----
> >  2 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> > index df56e4c..22af807 100644
> > --- a/drivers/iommu/Kconfig
> > +++ b/drivers/iommu/Kconfig
> > @@ -178,16 +178,15 @@ config TEGRA_IOMMU_SMMU
> >
> >  config EXYNOS_IOMMU
> >         bool "Exynos IOMMU Support"
> > -       depends on ARCH_EXYNOS && EXYNOS_DEV_SYSMMU
> > +       depends on ARCH_EXYNOS
> >         select IOMMU_API
> > +       default n
> 
> This is not needed as it is the default choice.
> 

OK.

> >         help
> >           Support for the IOMMU(System MMU) of Samsung Exynos application
> >           processor family. This enables H/W multimedia accellerators to see
> >           non-linear physical memory chunks as a linear memory in their
> >           address spaces
> >
> > -         If unsure, say N here.
> > -
> >  config EXYNOS_IOMMU_DEBUG
> >         bool "Debugging log for Exynos IOMMU"
> >         depends on EXYNOS_IOMMU
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 33b424d..34feb04 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -26,6 +26,7 @@
> >  #include <linux/list.h>
> >  #include <linux/memblock.h>
> >  #include <linux/export.h>
> > +#include <linux/of.h>
> >
> >  #include <asm/cacheflush.h>
> >  #include <asm/pgtable.h>
> > @@ -497,7 +498,7 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
> >         read_unlock_irqrestore(&data->lock, flags);
> >  }
> >
> > -static int exynos_sysmmu_probe(struct platform_device *pdev)
> > +static int __init exynos_sysmmu_probe(struct platform_device *pdev)
> >  {
> >         int irq, ret;
> >         struct device *dev = &pdev->dev;
> > @@ -557,11 +558,23 @@ static int exynos_sysmmu_probe(struct platform_device *pdev)
> >         return 0;
> >  }
> >
> > -static struct platform_driver exynos_sysmmu_driver = {
> > -       .probe          = exynos_sysmmu_probe,
> > -       .driver         = {
> > +#ifdef CONFIG_OF
> 
> This is not needed as Exynos is DT only platform.
> 
Yes, but I hesitate to remove that.

> > +static struct of_device_id sysmmu_of_match[] __initconst = {
> > +       { .compatible   = "samsung,sysmmu-v1", },
> > +       { .compatible   = "samsung,sysmmu-v2", },
> > +       { .compatible   = "samsung,sysmmu-v3.1", },
> > +       { .compatible   = "samsung,sysmmu-v3.2", },
> > +       { .compatible   = "samsung,sysmmu-v3.3", },
> > +       { },
> > +};
> > +#endif
> > +
> > +static struct platform_driver exynos_sysmmu_driver __refdata = {
> > +       .probe  = exynos_sysmmu_probe,
> > +       .driver = {
> >                 .owner          = THIS_MODULE,
> >                 .name           = "exynos-sysmmu",
> > +               .of_match_table = of_match_ptr(sysmmu_of_match),
> 
> of_match_ptr is not needed for the same reason as above.
> 
Ditto.

Thanks.

KyongHo

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Sachin Kamat]

On 18 March 2014 16:44, Cho KyongHo <pullip.cho@samsung.com> wrote:
> On Fri, 14 Mar 2014 20:57:42 +0530, Sachin Kamat wrote:
>> Hi KyongHo,
>>
>> On 14 March 2014 10:39, Cho KyongHo <pullip.cho@samsung.com> wrote:
>> >
>> > -static struct platform_driver exynos_sysmmu_driver = {
>> > -       .probe          = exynos_sysmmu_probe,
>> > -       .driver         = {
>> > +#ifdef CONFIG_OF
>>
>> This is not needed as Exynos is DT only platform.
>>
> Yes, but I hesitate to remove that.

What is the reason for your hesitation? :)


-- 
With warm regards,
Sachin

Re: [PATCH v11 13/27] iommu/exynos: support for device tree

[Cho KyongHo]

On Tue, 18 Mar 2014 16:46:24 +0530, Sachin Kamat wrote:
> On 18 March 2014 16:44, Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> > On Fri, 14 Mar 2014 20:57:42 +0530, Sachin Kamat wrote:
> >> Hi KyongHo,
> >>
> >> On 14 March 2014 10:39, Cho KyongHo <pullip.cho-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> >> >
> >> > -static struct platform_driver exynos_sysmmu_driver = {
> >> > -       .probe          = exynos_sysmmu_probe,
> >> > -       .driver         = {
> >> > +#ifdef CONFIG_OF
> >>
> >> This is not needed as Exynos is DT only platform.
> >>
> > Yes, but I hesitate to remove that.
> 
> What is the reason for your hesitation? :)
> 
Actually, no reason because you mentioned that Exynos is DT only platofrm.

I will remove that.

Thank you.

KyongHo