RE: [PATCH v3 13/13] OMAP: PM: DMA: Enable runtime pm

2010-11-10 Thread G, Manjunath Kondaiah


 -Original Message-
 From: linux-omap-ow...@vger.kernel.org 
 [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kevin Hilman
 Sent: Wednesday, November 10, 2010 4:18 AM
 To: G, Manjunath Kondaiah
 Cc: linux-omap@vger.kernel.org; 
 linux-arm-ker...@lists.infradead.org; Cousson, Benoit; 
 Shilimkar, Santosh
 Subject: Re: [PATCH v3 13/13] OMAP: PM: DMA: Enable runtime pm
 
 G, Manjunath Kondaiah manj...@ti.com writes:
 
  Enable runtime pm and use pm_runtime_get and pm_runtime_put
  for OMAP DMA driver.
 
  Signed-off-by: G, Manjunath Kondaiah manj...@ti.com
  Cc: Benoit Cousson b-cous...@ti.com
  Cc: Kevin Hilman khil...@deeprootsystems.com
  Cc: Santosh Shilimkar santosh.shilim...@ti.com
  ---
   arch/arm/plat-omap/dma.c |   13 +
   1 files changed, 13 insertions(+), 0 deletions(-)
 
  diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
  index 41b14b0..feac7ee 100644
  --- a/arch/arm/plat-omap/dma.c
  +++ b/arch/arm/plat-omap/dma.c
  @@ -35,6 +35,7 @@
   #include linux/io.h
   #include linux/slab.h
   #include linux/delay.h
  +#include linux/pm_runtime.h
   
   #include asm/system.h
   #include mach/hardware.h
  @@ -367,6 +368,8 @@ int omap_request_dma(int dev_id, const 
 char *dev_name,
  chan = dma_chan + free_ch;
  chan-dev_id = dev_id;
   
  +   pm_runtime_get(pd-dev);
 
 The _get() call is asynchronous.  So if the device was actually
 idled/disabled, immediately after this call it may still be
 idle/disabled.   When using the asynchronous versions of the API, the
 device should not be touched until the driver's -runtime_resume()
 callback is called.

driver runtime_resume will call omap_device_enable. How do we make 
we check from driver side, whether omap_device_enable execution is complete
or not.

 
  if (d-dev_caps  IS_WORD_16)
  p-clear_lch_regs(free_ch);
  else
  @@ -452,6 +455,7 @@ void omap_free_dma(int lch)
  omap_clear_dma(lch);
  p-clear_dma_sglist_mode(lch);
  }
  +   pm_runtime_put(pd-dev);
  spin_lock_irqsave(dma_chan_lock, flags);
  dma_chan[lch].dev_id = -1;
  dma_chan[lch].next_lch = -1;
  @@ -819,6 +823,11 @@ static int __devinit 
 omap_system_dma_probe(struct platform_device *pdev)
  dma_chan_count  = d-chan_count;
  dma_chan= d-chan;
   
  +   /* Enable run time PM */
 
 comment is redundant

ok.

 
  +   pm_runtime_enable(pd-dev);
  +
  +   /* Accessing hw registers, get clock */
 
 comment is wrong, driver doesn't know anything about clocks.
 Just drop comment as it doesn't tell you anything that the following
 'get' doesn't.
 
  +   pm_runtime_get(pd-dev);
 
 see above Re: async calls.
 
  for (ch = 0; ch  dma_chan_count; ch++) {
  unsigned long flags;
  omap_clear_dma(ch);
  @@ -846,6 +855,10 @@ static int __devinit 
 omap_system_dma_probe(struct platform_device *pdev)
  dma_chan[0].dev_id = 0;
  dma_chan[1].dev_id = 1;
  }
  +
  +   if (!omap_dma_reserve_channels)
  +   pm_runtime_put(pd-dev);
  +
 
 So if the reserve channels feature is used, PM is essentially disabled
 for sDMA.  You should add a warning here to that effect as well.

I will add warning comment.

-Manjunath--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 13/13] OMAP: PM: DMA: Enable runtime pm

2010-11-10 Thread Kevin Hilman
G, Manjunath Kondaiah manj...@ti.com writes:

 G, Manjunath Kondaiah manj...@ti.com writes:
 
  Enable runtime pm and use pm_runtime_get and pm_runtime_put
  for OMAP DMA driver.
 
  Signed-off-by: G, Manjunath Kondaiah manj...@ti.com
  Cc: Benoit Cousson b-cous...@ti.com
  Cc: Kevin Hilman khil...@deeprootsystems.com
  Cc: Santosh Shilimkar santosh.shilim...@ti.com
  ---
   arch/arm/plat-omap/dma.c |   13 +
   1 files changed, 13 insertions(+), 0 deletions(-)
 
  diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
  index 41b14b0..feac7ee 100644
  --- a/arch/arm/plat-omap/dma.c
  +++ b/arch/arm/plat-omap/dma.c
  @@ -35,6 +35,7 @@
   #include linux/io.h
   #include linux/slab.h
   #include linux/delay.h
  +#include linux/pm_runtime.h
   
   #include asm/system.h
   #include mach/hardware.h
  @@ -367,6 +368,8 @@ int omap_request_dma(int dev_id, const 
 char *dev_name,
 chan = dma_chan + free_ch;
 chan-dev_id = dev_id;
   
  +  pm_runtime_get(pd-dev);
 
 The _get() call is asynchronous.  So if the device was actually
 idled/disabled, immediately after this call it may still be
 idle/disabled.   When using the asynchronous versions of the API, the
 device should not be touched until the driver's -runtime_resume()
 callback is called.

 driver runtime_resume will call omap_device_enable. How do we make 
 we check from driver side, whether omap_device_enable execution is complete
 or not.

It is complete when the driver's runtime_resume hook is called.  The
OMAP runtime PM core calls omap_device_enable() before calling the
driver's callback.  See arch/arm/mach-omap2/pm_bus.c for details.

Alternatively, just use _get_sync() which doesn't return until the
device is ready.

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 13/13] OMAP: PM: DMA: Enable runtime pm

2010-11-09 Thread Kevin Hilman
G, Manjunath Kondaiah manj...@ti.com writes:

 Enable runtime pm and use pm_runtime_get and pm_runtime_put
 for OMAP DMA driver.

 Signed-off-by: G, Manjunath Kondaiah manj...@ti.com
 Cc: Benoit Cousson b-cous...@ti.com
 Cc: Kevin Hilman khil...@deeprootsystems.com
 Cc: Santosh Shilimkar santosh.shilim...@ti.com
 ---
  arch/arm/plat-omap/dma.c |   13 +
  1 files changed, 13 insertions(+), 0 deletions(-)

 diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
 index 41b14b0..feac7ee 100644
 --- a/arch/arm/plat-omap/dma.c
 +++ b/arch/arm/plat-omap/dma.c
 @@ -35,6 +35,7 @@
  #include linux/io.h
  #include linux/slab.h
  #include linux/delay.h
 +#include linux/pm_runtime.h
  
  #include asm/system.h
  #include mach/hardware.h
 @@ -367,6 +368,8 @@ int omap_request_dma(int dev_id, const char *dev_name,
   chan = dma_chan + free_ch;
   chan-dev_id = dev_id;
  
 + pm_runtime_get(pd-dev);

The _get() call is asynchronous.  So if the device was actually
idled/disabled, immediately after this call it may still be
idle/disabled.   When using the asynchronous versions of the API, the
device should not be touched until the driver's -runtime_resume()
callback is called.

   if (d-dev_caps  IS_WORD_16)
   p-clear_lch_regs(free_ch);
   else
 @@ -452,6 +455,7 @@ void omap_free_dma(int lch)
   omap_clear_dma(lch);
   p-clear_dma_sglist_mode(lch);
   }
 + pm_runtime_put(pd-dev);
   spin_lock_irqsave(dma_chan_lock, flags);
   dma_chan[lch].dev_id = -1;
   dma_chan[lch].next_lch = -1;
 @@ -819,6 +823,11 @@ static int __devinit omap_system_dma_probe(struct 
 platform_device *pdev)
   dma_chan_count  = d-chan_count;
   dma_chan= d-chan;
  
 + /* Enable run time PM */

comment is redundant

 + pm_runtime_enable(pd-dev);
 +
 + /* Accessing hw registers, get clock */

comment is wrong, driver doesn't know anything about clocks.
Just drop comment as it doesn't tell you anything that the following
'get' doesn't.

 + pm_runtime_get(pd-dev);

see above Re: async calls.

   for (ch = 0; ch  dma_chan_count; ch++) {
   unsigned long flags;
   omap_clear_dma(ch);
 @@ -846,6 +855,10 @@ static int __devinit omap_system_dma_probe(struct 
 platform_device *pdev)
   dma_chan[0].dev_id = 0;
   dma_chan[1].dev_id = 1;
   }
 +
 + if (!omap_dma_reserve_channels)
 + pm_runtime_put(pd-dev);
 +

So if the reserve channels feature is used, PM is essentially disabled
for sDMA.  You should add a warning here to that effect as well.


   return 0;
  
  exit_release_region:

Kevin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 13/13] OMAP: PM: DMA: Enable runtime pm

2010-10-26 Thread G, Manjunath Kondaiah
Enable runtime pm and use pm_runtime_get and pm_runtime_put
for OMAP DMA driver.

Signed-off-by: G, Manjunath Kondaiah manj...@ti.com
Cc: Benoit Cousson b-cous...@ti.com
Cc: Kevin Hilman khil...@deeprootsystems.com
Cc: Santosh Shilimkar santosh.shilim...@ti.com
---
 arch/arm/plat-omap/dma.c |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/plat-omap/dma.c b/arch/arm/plat-omap/dma.c
index 41b14b0..feac7ee 100644
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -35,6 +35,7 @@
 #include linux/io.h
 #include linux/slab.h
 #include linux/delay.h
+#include linux/pm_runtime.h
 
 #include asm/system.h
 #include mach/hardware.h
@@ -367,6 +368,8 @@ int omap_request_dma(int dev_id, const char *dev_name,
chan = dma_chan + free_ch;
chan-dev_id = dev_id;
 
+   pm_runtime_get(pd-dev);
+
if (d-dev_caps  IS_WORD_16)
p-clear_lch_regs(free_ch);
else
@@ -452,6 +455,7 @@ void omap_free_dma(int lch)
omap_clear_dma(lch);
p-clear_dma_sglist_mode(lch);
}
+   pm_runtime_put(pd-dev);
spin_lock_irqsave(dma_chan_lock, flags);
dma_chan[lch].dev_id = -1;
dma_chan[lch].next_lch = -1;
@@ -819,6 +823,11 @@ static int __devinit omap_system_dma_probe(struct 
platform_device *pdev)
dma_chan_count  = d-chan_count;
dma_chan= d-chan;
 
+   /* Enable run time PM */
+   pm_runtime_enable(pd-dev);
+
+   /* Accessing hw registers, get clock */
+   pm_runtime_get(pd-dev);
for (ch = 0; ch  dma_chan_count; ch++) {
unsigned long flags;
omap_clear_dma(ch);
@@ -846,6 +855,10 @@ static int __devinit omap_system_dma_probe(struct 
platform_device *pdev)
dma_chan[0].dev_id = 0;
dma_chan[1].dev_id = 1;
}
+
+   if (!omap_dma_reserve_channels)
+   pm_runtime_put(pd-dev);
+
return 0;
 
 exit_release_region:
-- 
1.7.1

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