Re: [PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-04 Thread Matt Porter
On Thu, Oct 04, 2012 at 05:18:41PM +0530, Sekhar Nori wrote:
> On 10/3/2012 8:25 PM, Matt Porter wrote:
> > From: Ben Gardiner 
> > 
> > The current davinci init sets up SRAM in iotables. There has been an 
> > observed
> > failure to boot a da850 with 128K specified in the iotable.
> > 
> > Make the davinci sram allocator -- now based on RMK's consolidated SRAM
> > support -- do an ioremap of the region specified by the entries in
> 
> The part about being based on RMK's consolidated SRAM support should be
> dropped.

Ok, cruft from trying to preserve Ben's original patch comments. Will
remove.

 
> > davinci_soc_info before registering with gen_pool_add_virt().
> > 
> > This commit breaks runtime of davinci boards since the regions that
> > the sram init is now trying to ioremap have been iomapped by their
> > iotable entries. The iotable entries will be removed in the patches
> > to come.
> 
> I would prefer merging 2/6 into this for this reason.

Ok, makes sense. This again comes from me trying to just use the
original patches. I'll squash these together for v4.

> 
> > 
> > Signed-off-by: Ben Gardiner 
> > [rebased to mainline as the consolidated SRAM support was dropped]
> > Signed-off-by: Matt Porter 
> > ---
> >  arch/arm/mach-davinci/sram.c |   17 ++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
> > index db0f778..0e8ca4f 100644
> > --- a/arch/arm/mach-davinci/sram.c
> > +++ b/arch/arm/mach-davinci/sram.c
> > @@ -10,6 +10,7 @@
> >   */
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  #include 
> > @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
> > return NULL;
> >  
> > if (dma)
> > -   *dma = dma_base + (vaddr - SRAM_VIRT);
> > +   *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
> > return (void *)vaddr;
> >  
> >  }
> > @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
> >   */
> >  static int __init sram_init(void)
> >  {
> > +   phys_addr_t phys = davinci_soc_info.sram_dma;
> > unsigned len = davinci_soc_info.sram_len;
> > int status = 0;
> > +   void *addr;
> >  
> > if (len) {
> > len = min_t(unsigned, len, SRAM_SIZE);
> > @@ -62,8 +65,16 @@ static int __init sram_init(void)
> > if (!sram_pool)
> > status = -ENOMEM;
> > }
> > -   if (sram_pool)
> > -   status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
> > +
> > +   if (sram_pool) {
> > +   addr = ioremap(phys, len);
> > +   if (!addr)
> > +   return -ENOMEM;
> > +   if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
> > +  phys, len, -1)))
> 
> Nit: prefer to set status outside of if().

Ok. Will change.

> 
> Looks good otherwise. Thanks for reviving this.

No problem, I'm just glad we seem to have a workable solution now.

Also, if you could check the conversation with Phillip about
gen_pool_find_by_phys(), I wonder what you think about depending on that
now, or waiting to convert to it when it goes upstream.

I'm torn as I see all the pdata going away anyway when Davinci is fully
converted to DT, and then also any use of find_by_phys() would go away
in favor of using the of helpers with his driver.

-Matt
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-04 Thread Sekhar Nori
On 10/3/2012 8:25 PM, Matt Porter wrote:
> From: Ben Gardiner 
> 
> The current davinci init sets up SRAM in iotables. There has been an observed
> failure to boot a da850 with 128K specified in the iotable.
> 
> Make the davinci sram allocator -- now based on RMK's consolidated SRAM
> support -- do an ioremap of the region specified by the entries in

The part about being based on RMK's consolidated SRAM support should be
dropped.

> davinci_soc_info before registering with gen_pool_add_virt().
> 
> This commit breaks runtime of davinci boards since the regions that
> the sram init is now trying to ioremap have been iomapped by their
> iotable entries. The iotable entries will be removed in the patches
> to come.

I would prefer merging 2/6 into this for this reason.

> 
> Signed-off-by: Ben Gardiner 
> [rebased to mainline as the consolidated SRAM support was dropped]
> Signed-off-by: Matt Porter 
> ---
>  arch/arm/mach-davinci/sram.c |   17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
> index db0f778..0e8ca4f 100644
> --- a/arch/arm/mach-davinci/sram.c
> +++ b/arch/arm/mach-davinci/sram.c
> @@ -10,6 +10,7 @@
>   */
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
>   return NULL;
>  
>   if (dma)
> - *dma = dma_base + (vaddr - SRAM_VIRT);
> + *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
>   return (void *)vaddr;
>  
>  }
> @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
>   */
>  static int __init sram_init(void)
>  {
> + phys_addr_t phys = davinci_soc_info.sram_dma;
>   unsigned len = davinci_soc_info.sram_len;
>   int status = 0;
> + void *addr;
>  
>   if (len) {
>   len = min_t(unsigned, len, SRAM_SIZE);
> @@ -62,8 +65,16 @@ static int __init sram_init(void)
>   if (!sram_pool)
>   status = -ENOMEM;
>   }
> - if (sram_pool)
> - status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
> +
> + if (sram_pool) {
> + addr = ioremap(phys, len);
> + if (!addr)
> + return -ENOMEM;
> + if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
> +phys, len, -1)))

Nit: prefer to set status outside of if().

Looks good otherwise. Thanks for reviving this.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-04 Thread Sekhar Nori
On 10/3/2012 8:25 PM, Matt Porter wrote:
 From: Ben Gardiner bengardi...@nanometrics.ca
 
 The current davinci init sets up SRAM in iotables. There has been an observed
 failure to boot a da850 with 128K specified in the iotable.
 
 Make the davinci sram allocator -- now based on RMK's consolidated SRAM
 support -- do an ioremap of the region specified by the entries in

The part about being based on RMK's consolidated SRAM support should be
dropped.

 davinci_soc_info before registering with gen_pool_add_virt().
 
 This commit breaks runtime of davinci boards since the regions that
 the sram init is now trying to ioremap have been iomapped by their
 iotable entries. The iotable entries will be removed in the patches
 to come.

I would prefer merging 2/6 into this for this reason.

 
 Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
 [rebased to mainline as the consolidated SRAM support was dropped]
 Signed-off-by: Matt Porter mpor...@ti.com
 ---
  arch/arm/mach-davinci/sram.c |   17 ++---
  1 file changed, 14 insertions(+), 3 deletions(-)
 
 diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
 index db0f778..0e8ca4f 100644
 --- a/arch/arm/mach-davinci/sram.c
 +++ b/arch/arm/mach-davinci/sram.c
 @@ -10,6 +10,7 @@
   */
  #include linux/module.h
  #include linux/init.h
 +#include linux/io.h
  #include linux/genalloc.h
  
  #include mach/common.h
 @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
   return NULL;
  
   if (dma)
 - *dma = dma_base + (vaddr - SRAM_VIRT);
 + *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
   return (void *)vaddr;
  
  }
 @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
   */
  static int __init sram_init(void)
  {
 + phys_addr_t phys = davinci_soc_info.sram_dma;
   unsigned len = davinci_soc_info.sram_len;
   int status = 0;
 + void *addr;
  
   if (len) {
   len = min_t(unsigned, len, SRAM_SIZE);
 @@ -62,8 +65,16 @@ static int __init sram_init(void)
   if (!sram_pool)
   status = -ENOMEM;
   }
 - if (sram_pool)
 - status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
 +
 + if (sram_pool) {
 + addr = ioremap(phys, len);
 + if (!addr)
 + return -ENOMEM;
 + if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
 +phys, len, -1)))

Nit: prefer to set status outside of if().

Looks good otherwise. Thanks for reviving this.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-04 Thread Matt Porter
On Thu, Oct 04, 2012 at 05:18:41PM +0530, Sekhar Nori wrote:
 On 10/3/2012 8:25 PM, Matt Porter wrote:
  From: Ben Gardiner bengardi...@nanometrics.ca
  
  The current davinci init sets up SRAM in iotables. There has been an 
  observed
  failure to boot a da850 with 128K specified in the iotable.
  
  Make the davinci sram allocator -- now based on RMK's consolidated SRAM
  support -- do an ioremap of the region specified by the entries in
 
 The part about being based on RMK's consolidated SRAM support should be
 dropped.

Ok, cruft from trying to preserve Ben's original patch comments. Will
remove.

 
  davinci_soc_info before registering with gen_pool_add_virt().
  
  This commit breaks runtime of davinci boards since the regions that
  the sram init is now trying to ioremap have been iomapped by their
  iotable entries. The iotable entries will be removed in the patches
  to come.
 
 I would prefer merging 2/6 into this for this reason.

Ok, makes sense. This again comes from me trying to just use the
original patches. I'll squash these together for v4.

 
  
  Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
  [rebased to mainline as the consolidated SRAM support was dropped]
  Signed-off-by: Matt Porter mpor...@ti.com
  ---
   arch/arm/mach-davinci/sram.c |   17 ++---
   1 file changed, 14 insertions(+), 3 deletions(-)
  
  diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
  index db0f778..0e8ca4f 100644
  --- a/arch/arm/mach-davinci/sram.c
  +++ b/arch/arm/mach-davinci/sram.c
  @@ -10,6 +10,7 @@
*/
   #include linux/module.h
   #include linux/init.h
  +#include linux/io.h
   #include linux/genalloc.h
   
   #include mach/common.h
  @@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
  return NULL;
   
  if (dma)
  -   *dma = dma_base + (vaddr - SRAM_VIRT);
  +   *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
  return (void *)vaddr;
   
   }
  @@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
*/
   static int __init sram_init(void)
   {
  +   phys_addr_t phys = davinci_soc_info.sram_dma;
  unsigned len = davinci_soc_info.sram_len;
  int status = 0;
  +   void *addr;
   
  if (len) {
  len = min_t(unsigned, len, SRAM_SIZE);
  @@ -62,8 +65,16 @@ static int __init sram_init(void)
  if (!sram_pool)
  status = -ENOMEM;
  }
  -   if (sram_pool)
  -   status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
  +
  +   if (sram_pool) {
  +   addr = ioremap(phys, len);
  +   if (!addr)
  +   return -ENOMEM;
  +   if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
  +  phys, len, -1)))
 
 Nit: prefer to set status outside of if().

Ok. Will change.

 
 Looks good otherwise. Thanks for reviving this.

No problem, I'm just glad we seem to have a workable solution now.

Also, if you could check the conversation with Phillip about
gen_pool_find_by_phys(), I wonder what you think about depending on that
now, or waiting to convert to it when it goes upstream.

I'm torn as I see all the pdata going away anyway when Davinci is fully
converted to DT, and then also any use of find_by_phys() would go away
in favor of using the of helpers with his driver.

-Matt
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-03 Thread Matt Porter
From: Ben Gardiner 

The current davinci init sets up SRAM in iotables. There has been an observed
failure to boot a da850 with 128K specified in the iotable.

Make the davinci sram allocator -- now based on RMK's consolidated SRAM
support -- do an ioremap of the region specified by the entries in
davinci_soc_info before registering with gen_pool_add_virt().

This commit breaks runtime of davinci boards since the regions that
the sram init is now trying to ioremap have been iomapped by their
iotable entries. The iotable entries will be removed in the patches
to come.

Signed-off-by: Ben Gardiner 
[rebased to mainline as the consolidated SRAM support was dropped]
Signed-off-by: Matt Porter 
---
 arch/arm/mach-davinci/sram.c |   17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
index db0f778..0e8ca4f 100644
--- a/arch/arm/mach-davinci/sram.c
+++ b/arch/arm/mach-davinci/sram.c
@@ -10,6 +10,7 @@
  */
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
return NULL;
 
if (dma)
-   *dma = dma_base + (vaddr - SRAM_VIRT);
+   *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
return (void *)vaddr;
 
 }
@@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
  */
 static int __init sram_init(void)
 {
+   phys_addr_t phys = davinci_soc_info.sram_dma;
unsigned len = davinci_soc_info.sram_len;
int status = 0;
+   void *addr;
 
if (len) {
len = min_t(unsigned, len, SRAM_SIZE);
@@ -62,8 +65,16 @@ static int __init sram_init(void)
if (!sram_pool)
status = -ENOMEM;
}
-   if (sram_pool)
-   status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
+
+   if (sram_pool) {
+   addr = ioremap(phys, len);
+   if (!addr)
+   return -ENOMEM;
+   if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
+  phys, len, -1)))
+   iounmap(addr);
+   }
+
WARN_ON(status < 0);
return status;
 }
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v3 1/6] ARM: davinci: sram: ioremap the davinci_soc_info specified sram regions

2012-10-03 Thread Matt Porter
From: Ben Gardiner bengardi...@nanometrics.ca

The current davinci init sets up SRAM in iotables. There has been an observed
failure to boot a da850 with 128K specified in the iotable.

Make the davinci sram allocator -- now based on RMK's consolidated SRAM
support -- do an ioremap of the region specified by the entries in
davinci_soc_info before registering with gen_pool_add_virt().

This commit breaks runtime of davinci boards since the regions that
the sram init is now trying to ioremap have been iomapped by their
iotable entries. The iotable entries will be removed in the patches
to come.

Signed-off-by: Ben Gardiner bengardi...@nanometrics.ca
[rebased to mainline as the consolidated SRAM support was dropped]
Signed-off-by: Matt Porter mpor...@ti.com
---
 arch/arm/mach-davinci/sram.c |   17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/sram.c b/arch/arm/mach-davinci/sram.c
index db0f778..0e8ca4f 100644
--- a/arch/arm/mach-davinci/sram.c
+++ b/arch/arm/mach-davinci/sram.c
@@ -10,6 +10,7 @@
  */
 #include linux/module.h
 #include linux/init.h
+#include linux/io.h
 #include linux/genalloc.h
 
 #include mach/common.h
@@ -32,7 +33,7 @@ void *sram_alloc(size_t len, dma_addr_t *dma)
return NULL;
 
if (dma)
-   *dma = dma_base + (vaddr - SRAM_VIRT);
+   *dma = gen_pool_virt_to_phys(sram_pool, vaddr);
return (void *)vaddr;
 
 }
@@ -53,8 +54,10 @@ EXPORT_SYMBOL(sram_free);
  */
 static int __init sram_init(void)
 {
+   phys_addr_t phys = davinci_soc_info.sram_dma;
unsigned len = davinci_soc_info.sram_len;
int status = 0;
+   void *addr;
 
if (len) {
len = min_t(unsigned, len, SRAM_SIZE);
@@ -62,8 +65,16 @@ static int __init sram_init(void)
if (!sram_pool)
status = -ENOMEM;
}
-   if (sram_pool)
-   status = gen_pool_add(sram_pool, SRAM_VIRT, len, -1);
+
+   if (sram_pool) {
+   addr = ioremap(phys, len);
+   if (!addr)
+   return -ENOMEM;
+   if((status = gen_pool_add_virt(sram_pool, (unsigned)addr,
+  phys, len, -1)))
+   iounmap(addr);
+   }
+
WARN_ON(status  0);
return status;
 }
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/