Patchwork [1/2] ARM: imx: add dt support of IRAM

login
register
mail settings
Submitter Jason Chen
Date Dec. 27, 2011, 1:33 a.m.
Message ID <1324949596-27214-1-git-send-email-jason.chen@linaro.org>
Download mbox | patch
Permalink /patch/133292/
State New
Headers show

Comments

Jason Chen - Dec. 27, 2011, 1:33 a.m.
Signed-off-by: Jason Chen <jason.chen@linaro.org>
Signed-off-by: Eric Miao <eric.miao@linaro.org>
---
 arch/arm/plat-mxc/include/mach/iram.h |    6 ++++++
 arch/arm/plat-mxc/iram_alloc.c        |   16 ++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)
Shawn Guo - Dec. 27, 2011, 7:02 a.m.
On Tue, Dec 27, 2011 at 09:33:16AM +0800, Jason Chen wrote:
> Signed-off-by: Jason Chen <jason.chen@linaro.org>
> Signed-off-by: Eric Miao <eric.miao@linaro.org>
> ---
>  arch/arm/plat-mxc/include/mach/iram.h |    6 ++++++
>  arch/arm/plat-mxc/iram_alloc.c        |   16 ++++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/iram.h b/arch/arm/plat-mxc/include/mach/iram.h
> index 022690c..f8372cf 100644
> --- a/arch/arm/plat-mxc/include/mach/iram.h
> +++ b/arch/arm/plat-mxc/include/mach/iram.h
> @@ -21,6 +21,7 @@
>  #ifdef CONFIG_IRAM_ALLOC
>  
>  int __init iram_init(unsigned long base, unsigned long size);
> +int __init of_iram_init(void);

It may be inherited from iram_init declaration on the above line.  But
it's really unnecessary to have '__init' for function declaration.

And I suggested the function name iram_of_init than of_iram_init for
some reason.  Looking at include/linux/of.h, you will find that all DT
core functions use naming convention of_xxx.  To differentiate from DT
core functions, we want to use iram_of_init just like what gic driver
does (gic_of_init in arch/arm/common/gic.c).

>  void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr);
>  void iram_free(unsigned long dma_addr, unsigned int size);
>  
> @@ -31,6 +32,11 @@ static inline int __init iram_init(unsigned long base, unsigned long size)
>  	return -ENOMEM;
>  }
>  
> +static inline int __init of_iram_init(void)
> +{
> +	return -EINVAL;
> +}
> +
>  static inline void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr)
>  {
>  	return NULL;
> diff --git a/arch/arm/plat-mxc/iram_alloc.c b/arch/arm/plat-mxc/iram_alloc.c
> index 074c386..f73ca9d 100644
> --- a/arch/arm/plat-mxc/iram_alloc.c
> +++ b/arch/arm/plat-mxc/iram_alloc.c
> @@ -22,6 +22,8 @@
>  #include <linux/module.h>
>  #include <linux/spinlock.h>
>  #include <linux/genalloc.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <mach/iram.h>
>  
>  static unsigned long iram_phys_base;
> @@ -71,3 +73,17 @@ int __init iram_init(unsigned long base, unsigned long size)
>  	pr_debug("i.MX IRAM pool: %ld KB@0x%p\n", size / 1024, iram_virt_base);
>  	return 0;
>  }
> +
> +int __init of_iram_init(void)
> +{
> +	struct device_node *np;
> +	struct resource res;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,imx-iram");
> +	if (of_address_to_resource(np, 0, &res))
> +		return -EINVAL;
> +	if (res.start && (res.end > res.start))

It's an valid case that iram starts at physical address 0.  And how
can we run into !(res.end > res.start)?  To me, it's an unnecessary
checking at all.

> +		return iram_init(res.start, res.end - res.start + 1);

We can use resource_size(&res) to help here.

Regards,
Shawn

> +	else
> +		return -EINVAL;
> +}
> -- 
> 1.7.4.1
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Rob Herring - Dec. 27, 2011, 4:54 p.m.
On 12/26/2011 07:33 PM, Jason Chen wrote:
> Signed-off-by: Jason Chen <jason.chen@linaro.org>
> Signed-off-by: Eric Miao <eric.miao@linaro.org>
> ---
>  arch/arm/plat-mxc/include/mach/iram.h |    6 ++++++
>  arch/arm/plat-mxc/iram_alloc.c        |   16 ++++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)
> 

Whatever happened to the generic SRAM support? That with a common DT
binding and init would be better.

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-May/051472.html

Rob

> diff --git a/arch/arm/plat-mxc/include/mach/iram.h b/arch/arm/plat-mxc/include/mach/iram.h
> index 022690c..f8372cf 100644
> --- a/arch/arm/plat-mxc/include/mach/iram.h
> +++ b/arch/arm/plat-mxc/include/mach/iram.h
> @@ -21,6 +21,7 @@
>  #ifdef CONFIG_IRAM_ALLOC
>  
>  int __init iram_init(unsigned long base, unsigned long size);
> +int __init of_iram_init(void);
>  void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr);
>  void iram_free(unsigned long dma_addr, unsigned int size);
>  
> @@ -31,6 +32,11 @@ static inline int __init iram_init(unsigned long base, unsigned long size)
>  	return -ENOMEM;
>  }
>  
> +static inline int __init of_iram_init(void)
> +{
> +	return -EINVAL;
> +}
> +
>  static inline void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr)
>  {
>  	return NULL;
> diff --git a/arch/arm/plat-mxc/iram_alloc.c b/arch/arm/plat-mxc/iram_alloc.c
> index 074c386..f73ca9d 100644
> --- a/arch/arm/plat-mxc/iram_alloc.c
> +++ b/arch/arm/plat-mxc/iram_alloc.c
> @@ -22,6 +22,8 @@
>  #include <linux/module.h>
>  #include <linux/spinlock.h>
>  #include <linux/genalloc.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
>  #include <mach/iram.h>
>  
>  static unsigned long iram_phys_base;
> @@ -71,3 +73,17 @@ int __init iram_init(unsigned long base, unsigned long size)
>  	pr_debug("i.MX IRAM pool: %ld KB@0x%p\n", size / 1024, iram_virt_base);
>  	return 0;
>  }
> +
> +int __init of_iram_init(void)
> +{
> +	struct device_node *np;
> +	struct resource res;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,imx-iram");
> +	if (of_address_to_resource(np, 0, &res))
> +		return -EINVAL;
> +	if (res.start && (res.end > res.start))
> +		return iram_init(res.start, res.end - res.start + 1);
> +	else
> +		return -EINVAL;
> +}
Shawn Guo - Dec. 28, 2011, 6:27 a.m.
On Tue, Dec 27, 2011 at 10:54:54AM -0600, Rob Herring wrote:
> On 12/26/2011 07:33 PM, Jason Chen wrote:
> > Signed-off-by: Jason Chen <jason.chen@linaro.org>
> > Signed-off-by: Eric Miao <eric.miao@linaro.org>
> > ---
> >  arch/arm/plat-mxc/include/mach/iram.h |    6 ++++++
> >  arch/arm/plat-mxc/iram_alloc.c        |   16 ++++++++++++++++
> >  2 files changed, 22 insertions(+), 0 deletions(-)
> > 
> 
> Whatever happened to the generic SRAM support? That with a common DT
> binding and init would be better.
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-May/051472.html
> 
Receiving the patch from Eric and Jason, the first thing I did was
searching the status of SRAM consolidation series.  But it has not got
any update since it's proposed back to May.
Eric Miao - Dec. 28, 2011, 12:23 p.m.
On Wed, Dec 28, 2011 at 12:54 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 12/26/2011 07:33 PM, Jason Chen wrote:
>> Signed-off-by: Jason Chen <jason.chen@linaro.org>
>> Signed-off-by: Eric Miao <eric.miao@linaro.org>
>> ---
>>  arch/arm/plat-mxc/include/mach/iram.h |    6 ++++++
>>  arch/arm/plat-mxc/iram_alloc.c        |   16 ++++++++++++++++
>>  2 files changed, 22 insertions(+), 0 deletions(-)
>>
>
> Whatever happened to the generic SRAM support? That with a common DT
> binding and init would be better.
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2011-May/051472.html

+1, this one looks good.

Patch

diff --git a/arch/arm/plat-mxc/include/mach/iram.h b/arch/arm/plat-mxc/include/mach/iram.h
index 022690c..f8372cf 100644
--- a/arch/arm/plat-mxc/include/mach/iram.h
+++ b/arch/arm/plat-mxc/include/mach/iram.h
@@ -21,6 +21,7 @@ 
 #ifdef CONFIG_IRAM_ALLOC
 
 int __init iram_init(unsigned long base, unsigned long size);
+int __init of_iram_init(void);
 void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr);
 void iram_free(unsigned long dma_addr, unsigned int size);
 
@@ -31,6 +32,11 @@  static inline int __init iram_init(unsigned long base, unsigned long size)
 	return -ENOMEM;
 }
 
+static inline int __init of_iram_init(void)
+{
+	return -EINVAL;
+}
+
 static inline void __iomem *iram_alloc(unsigned int size, unsigned long *dma_addr)
 {
 	return NULL;
diff --git a/arch/arm/plat-mxc/iram_alloc.c b/arch/arm/plat-mxc/iram_alloc.c
index 074c386..f73ca9d 100644
--- a/arch/arm/plat-mxc/iram_alloc.c
+++ b/arch/arm/plat-mxc/iram_alloc.c
@@ -22,6 +22,8 @@ 
 #include <linux/module.h>
 #include <linux/spinlock.h>
 #include <linux/genalloc.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
 #include <mach/iram.h>
 
 static unsigned long iram_phys_base;
@@ -71,3 +73,17 @@  int __init iram_init(unsigned long base, unsigned long size)
 	pr_debug("i.MX IRAM pool: %ld KB@0x%p\n", size / 1024, iram_virt_base);
 	return 0;
 }
+
+int __init of_iram_init(void)
+{
+	struct device_node *np;
+	struct resource res;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,imx-iram");
+	if (of_address_to_resource(np, 0, &res))
+		return -EINVAL;
+	if (res.start && (res.end > res.start))
+		return iram_init(res.start, res.end - res.start + 1);
+	else
+		return -EINVAL;
+}