Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Thomas Petazzoni

On Thu, 25 Oct 2012 13:46:41 +, Arnd Bergmann wrote:

> > Seems like the driver is too lazy and allocates everything coherent
> > to avoid the hassle of doing dma_map/dma_unmap operations when
> > needed, but I haven't looked in details at the driver yet to see if
> > it would be possible to switch those DMA coherent allocations into
> > non-coherent allocations + appropriate calls to the DMA operations.
> 
> Using coherent allocations is fine, I was wondering whether they need
> to be atomic or not.

You're raising a good point here: all the dma_pool_alloc()
allocations done by sata_mv are GFP_KERNEL. So why are we having
problems with the /atomic/ coherent pool size? Is it the libata core
that's doing GFP_ATOMIC DMA coherent allocations. It doesn't seem so.
Something's odd.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Arnd Bergmann
On Thursday 25 October 2012, Thomas Petazzoni wrote:
> On Thu, 25 Oct 2012 11:27:36 +, Arnd Bergmann wrote:
> > On Wednesday 24 October 2012, Gregory CLEMENT wrote:
> > > For Armada 370/XP we have the same problem that for the commit
> > > cb01b63, so we applied the same solution: "The default 256 KiB
> > > coherent pool may be too small for some of the Kirkwood devices, so
> > > increase it to make sure that devices will be able to allocate their
> > > buffers with GFP_ATOMIC flag"
> > > 
> > > Signed-off-by: Gregory CLEMENT 
> > 
> > Do you know why the ATA driver needs this? I find it surprising that
> > it's necessary, so I'd like to make sure we're not just working around
> > a device driver bug here.
> 
> The sata_mv driver create dma_pool and allocate objects from them, and
> all the memory allocated for dma_pools is allocated using
> dma_alloc_coherent(), and I guess the driver is using too much of them.
> 
> Seems like the driver is too lazy and allocates everything coherent to
> avoid the hassle of doing dma_map/dma_unmap operations when needed, but
> I haven't looked in details at the driver yet to see if it would be
> possible to switch those DMA coherent allocations into non-coherent
> allocations + appropriate calls to the DMA operations.

Using coherent allocations is fine, I was wondering whether they need
to be atomic or not.

> That said, that's for sure a larger task than just enabling SATA on
> Armada 370/XP, so I would advocate to handle this problem separately.

Agreed.

Arnd
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Thomas Petazzoni
Arnd,

On Thu, 25 Oct 2012 11:27:36 +, Arnd Bergmann wrote:
> On Wednesday 24 October 2012, Gregory CLEMENT wrote:
> > For Armada 370/XP we have the same problem that for the commit
> > cb01b63, so we applied the same solution: "The default 256 KiB
> > coherent pool may be too small for some of the Kirkwood devices, so
> > increase it to make sure that devices will be able to allocate their
> > buffers with GFP_ATOMIC flag"
> > 
> > Signed-off-by: Gregory CLEMENT 
> 
> Do you know why the ATA driver needs this? I find it surprising that
> it's necessary, so I'd like to make sure we're not just working around
> a device driver bug here.

The sata_mv driver create dma_pool and allocate objects from them, and
all the memory allocated for dma_pools is allocated using
dma_alloc_coherent(), and I guess the driver is using too much of them.

Seems like the driver is too lazy and allocates everything coherent to
avoid the hassle of doing dma_map/dma_unmap operations when needed, but
I haven't looked in details at the driver yet to see if it would be
possible to switch those DMA coherent allocations into non-coherent
allocations + appropriate calls to the DMA operations.

That said, that's for sure a larger task than just enabling SATA on
Armada 370/XP, so I would advocate to handle this problem separately.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Arnd Bergmann
On Wednesday 24 October 2012, Gregory CLEMENT wrote:
> For Armada 370/XP we have the same problem that for the commit
> cb01b63, so we applied the same solution: "The default 256 KiB
> coherent pool may be too small for some of the Kirkwood devices, so
> increase it to make sure that devices will be able to allocate their
> buffers with GFP_ATOMIC flag"
> 
> Signed-off-by: Gregory CLEMENT 

Do you know why the ATA driver needs this? I find it surprising that
it's necessary, so I'd like to make sure we're not just working around
a device driver bug here.

Arnd
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Arnd Bergmann
On Wednesday 24 October 2012, Gregory CLEMENT wrote:
 For Armada 370/XP we have the same problem that for the commit
 cb01b63, so we applied the same solution: The default 256 KiB
 coherent pool may be too small for some of the Kirkwood devices, so
 increase it to make sure that devices will be able to allocate their
 buffers with GFP_ATOMIC flag
 
 Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com

Do you know why the ATA driver needs this? I find it surprising that
it's necessary, so I'd like to make sure we're not just working around
a device driver bug here.

Arnd
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Thomas Petazzoni
Arnd,

On Thu, 25 Oct 2012 11:27:36 +, Arnd Bergmann wrote:
 On Wednesday 24 October 2012, Gregory CLEMENT wrote:
  For Armada 370/XP we have the same problem that for the commit
  cb01b63, so we applied the same solution: The default 256 KiB
  coherent pool may be too small for some of the Kirkwood devices, so
  increase it to make sure that devices will be able to allocate their
  buffers with GFP_ATOMIC flag
  
  Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com
 
 Do you know why the ATA driver needs this? I find it surprising that
 it's necessary, so I'd like to make sure we're not just working around
 a device driver bug here.

The sata_mv driver create dma_pool and allocate objects from them, and
all the memory allocated for dma_pools is allocated using
dma_alloc_coherent(), and I guess the driver is using too much of them.

Seems like the driver is too lazy and allocates everything coherent to
avoid the hassle of doing dma_map/dma_unmap operations when needed, but
I haven't looked in details at the driver yet to see if it would be
possible to switch those DMA coherent allocations into non-coherent
allocations + appropriate calls to the DMA operations.

That said, that's for sure a larger task than just enabling SATA on
Armada 370/XP, so I would advocate to handle this problem separately.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Arnd Bergmann
On Thursday 25 October 2012, Thomas Petazzoni wrote:
 On Thu, 25 Oct 2012 11:27:36 +, Arnd Bergmann wrote:
  On Wednesday 24 October 2012, Gregory CLEMENT wrote:
   For Armada 370/XP we have the same problem that for the commit
   cb01b63, so we applied the same solution: The default 256 KiB
   coherent pool may be too small for some of the Kirkwood devices, so
   increase it to make sure that devices will be able to allocate their
   buffers with GFP_ATOMIC flag
   
   Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com
  
  Do you know why the ATA driver needs this? I find it surprising that
  it's necessary, so I'd like to make sure we're not just working around
  a device driver bug here.
 
 The sata_mv driver create dma_pool and allocate objects from them, and
 all the memory allocated for dma_pools is allocated using
 dma_alloc_coherent(), and I guess the driver is using too much of them.
 
 Seems like the driver is too lazy and allocates everything coherent to
 avoid the hassle of doing dma_map/dma_unmap operations when needed, but
 I haven't looked in details at the driver yet to see if it would be
 possible to switch those DMA coherent allocations into non-coherent
 allocations + appropriate calls to the DMA operations.

Using coherent allocations is fine, I was wondering whether they need
to be atomic or not.

 That said, that's for sure a larger task than just enabling SATA on
 Armada 370/XP, so I would advocate to handle this problem separately.

Agreed.

Arnd
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-25 Thread Thomas Petazzoni

On Thu, 25 Oct 2012 13:46:41 +, Arnd Bergmann wrote:

  Seems like the driver is too lazy and allocates everything coherent
  to avoid the hassle of doing dma_map/dma_unmap operations when
  needed, but I haven't looked in details at the driver yet to see if
  it would be possible to switch those DMA coherent allocations into
  non-coherent allocations + appropriate calls to the DMA operations.
 
 Using coherent allocations is fine, I was wondering whether they need
 to be atomic or not.

You're raising a good point here: all the dma_pool_alloc()
allocations done by sata_mv are GFP_KERNEL. So why are we having
problems with the /atomic/ coherent pool size? Is it the libata core
that's doing GFP_ATOMIC DMA coherent allocations. It doesn't seem so.
Something's odd.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-24 Thread Marek Szyprowski

Hello,

On 10/24/2012 3:49 PM, Gregory CLEMENT wrote:

For Armada 370/XP we have the same problem that for the commit
cb01b63, so we applied the same solution: "The default 256 KiB
coherent pool may be too small for some of the Kirkwood devices, so
increase it to make sure that devices will be able to allocate their
buffers with GFP_ATOMIC flag"

Signed-off-by: Gregory CLEMENT 

Cc: Marek Szyprowski 


Acked-by: Marek Szyprowski 


---
  arch/arm/mach-mvebu/armada-370-xp.c |   12 
  1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-mvebu/armada-370-xp.c 
b/arch/arm/mach-mvebu/armada-370-xp.c
index 2af6ce5..cbad821 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.c
+++ b/arch/arm/mach-mvebu/armada-370-xp.c
@@ -17,6 +17,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -43,6 +44,16 @@ void __init armada_370_xp_timer_and_clk_init(void)
armada_370_xp_timer_init();
  }

+void __init armada_370_xp_init_early(void)
+{
+   /*
+* Some Armada 370/XP devices allocate their coherent buffers
+* from atomic context. Increase size of atomic coherent pool
+* to make sure such the allocations won't fail.
+*/
+   init_dma_coherent_pool_size(SZ_1M);
+}
+
  struct sys_timer armada_370_xp_timer = {
.init   = armada_370_xp_timer_and_clk_init,
  };
@@ -61,6 +72,7 @@ static const char * const armada_370_xp_dt_board_dt_compat[] 
= {
  DT_MACHINE_START(ARMADA_XP_DT, "Marvell Aramada 370/XP (Device Tree)")
.init_machine   = armada_370_xp_dt_init,
.map_io = armada_370_xp_map_io,
+   .init_early = armada_370_xp_init_early,
.init_irq   = armada_370_xp_init_irq,
.handle_irq = armada_370_xp_handle_irq,
.timer  = _370_xp_timer,


Best regards
--
Marek Szyprowski
Samsung Poland R Center

--
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-24 Thread Gregory CLEMENT
For Armada 370/XP we have the same problem that for the commit
cb01b63, so we applied the same solution: "The default 256 KiB
coherent pool may be too small for some of the Kirkwood devices, so
increase it to make sure that devices will be able to allocate their
buffers with GFP_ATOMIC flag"

Signed-off-by: Gregory CLEMENT 

Cc: Marek Szyprowski 
---
 arch/arm/mach-mvebu/armada-370-xp.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-mvebu/armada-370-xp.c 
b/arch/arm/mach-mvebu/armada-370-xp.c
index 2af6ce5..cbad821 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.c
+++ b/arch/arm/mach-mvebu/armada-370-xp.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -43,6 +44,16 @@ void __init armada_370_xp_timer_and_clk_init(void)
armada_370_xp_timer_init();
 }
 
+void __init armada_370_xp_init_early(void)
+{
+   /*
+* Some Armada 370/XP devices allocate their coherent buffers
+* from atomic context. Increase size of atomic coherent pool
+* to make sure such the allocations won't fail.
+*/
+   init_dma_coherent_pool_size(SZ_1M);
+}
+
 struct sys_timer armada_370_xp_timer = {
.init   = armada_370_xp_timer_and_clk_init,
 };
@@ -61,6 +72,7 @@ static const char * const armada_370_xp_dt_board_dt_compat[] 
= {
 DT_MACHINE_START(ARMADA_XP_DT, "Marvell Aramada 370/XP (Device Tree)")
.init_machine   = armada_370_xp_dt_init,
.map_io = armada_370_xp_map_io,
+   .init_early = armada_370_xp_init_early,
.init_irq   = armada_370_xp_init_irq,
.handle_irq = armada_370_xp_handle_irq,
.timer  = _370_xp_timer,
-- 
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 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-24 Thread Gregory CLEMENT
For Armada 370/XP we have the same problem that for the commit
cb01b63, so we applied the same solution: The default 256 KiB
coherent pool may be too small for some of the Kirkwood devices, so
increase it to make sure that devices will be able to allocate their
buffers with GFP_ATOMIC flag

Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com

Cc: Marek Szyprowski m.szyprow...@samsung.com
---
 arch/arm/mach-mvebu/armada-370-xp.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-mvebu/armada-370-xp.c 
b/arch/arm/mach-mvebu/armada-370-xp.c
index 2af6ce5..cbad821 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.c
+++ b/arch/arm/mach-mvebu/armada-370-xp.c
@@ -17,6 +17,7 @@
 #include linux/of_platform.h
 #include linux/io.h
 #include linux/time-armada-370-xp.h
+#include linux/dma-mapping.h
 #include asm/mach/arch.h
 #include asm/mach/map.h
 #include asm/mach/time.h
@@ -43,6 +44,16 @@ void __init armada_370_xp_timer_and_clk_init(void)
armada_370_xp_timer_init();
 }
 
+void __init armada_370_xp_init_early(void)
+{
+   /*
+* Some Armada 370/XP devices allocate their coherent buffers
+* from atomic context. Increase size of atomic coherent pool
+* to make sure such the allocations won't fail.
+*/
+   init_dma_coherent_pool_size(SZ_1M);
+}
+
 struct sys_timer armada_370_xp_timer = {
.init   = armada_370_xp_timer_and_clk_init,
 };
@@ -61,6 +72,7 @@ static const char * const armada_370_xp_dt_board_dt_compat[] 
= {
 DT_MACHINE_START(ARMADA_XP_DT, Marvell Aramada 370/XP (Device Tree))
.init_machine   = armada_370_xp_dt_init,
.map_io = armada_370_xp_map_io,
+   .init_early = armada_370_xp_init_early,
.init_irq   = armada_370_xp_init_irq,
.handle_irq = armada_370_xp_handle_irq,
.timer  = armada_370_xp_timer,
-- 
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/


Re: [PATCH 1/2] arm: mvebu: increase atomic coherent pool size for armada 370/XP

2012-10-24 Thread Marek Szyprowski

Hello,

On 10/24/2012 3:49 PM, Gregory CLEMENT wrote:

For Armada 370/XP we have the same problem that for the commit
cb01b63, so we applied the same solution: The default 256 KiB
coherent pool may be too small for some of the Kirkwood devices, so
increase it to make sure that devices will be able to allocate their
buffers with GFP_ATOMIC flag

Signed-off-by: Gregory CLEMENT gregory.clem...@free-electrons.com

Cc: Marek Szyprowski m.szyprow...@samsung.com


Acked-by: Marek Szyprowski m.szyprow...@samsung.com


---
  arch/arm/mach-mvebu/armada-370-xp.c |   12 
  1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-mvebu/armada-370-xp.c 
b/arch/arm/mach-mvebu/armada-370-xp.c
index 2af6ce5..cbad821 100644
--- a/arch/arm/mach-mvebu/armada-370-xp.c
+++ b/arch/arm/mach-mvebu/armada-370-xp.c
@@ -17,6 +17,7 @@
  #include linux/of_platform.h
  #include linux/io.h
  #include linux/time-armada-370-xp.h
+#include linux/dma-mapping.h
  #include asm/mach/arch.h
  #include asm/mach/map.h
  #include asm/mach/time.h
@@ -43,6 +44,16 @@ void __init armada_370_xp_timer_and_clk_init(void)
armada_370_xp_timer_init();
  }

+void __init armada_370_xp_init_early(void)
+{
+   /*
+* Some Armada 370/XP devices allocate their coherent buffers
+* from atomic context. Increase size of atomic coherent pool
+* to make sure such the allocations won't fail.
+*/
+   init_dma_coherent_pool_size(SZ_1M);
+}
+
  struct sys_timer armada_370_xp_timer = {
.init   = armada_370_xp_timer_and_clk_init,
  };
@@ -61,6 +72,7 @@ static const char * const armada_370_xp_dt_board_dt_compat[] 
= {
  DT_MACHINE_START(ARMADA_XP_DT, Marvell Aramada 370/XP (Device Tree))
.init_machine   = armada_370_xp_dt_init,
.map_io = armada_370_xp_map_io,
+   .init_early = armada_370_xp_init_early,
.init_irq   = armada_370_xp_init_irq,
.handle_irq = armada_370_xp_handle_irq,
.timer  = armada_370_xp_timer,


Best regards
--
Marek Szyprowski
Samsung Poland RD Center

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