Re: [PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-11 Thread Cho KyongHo
On Fri, 09 Aug 2013 10:32:40 +0200, Tomasz Figa wrote:
> On Friday 09 of August 2013 16:49:43 Cho KyongHo wrote:
> > On Fri, 09 Aug 2013 01:03:05 +0200, Tomasz Figa wrote:
> > > Hi KyongHo,
> > > 
> > > nit: Please drop the trailing dot at the end of patch subject.
> > 
> > Oh. I didn't catch that.
> > Thank you.
> > 
> > > On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
> > > > This adds support for Advance Power Management and Runtime Power
> > > > Management.
> > > 
> > > This patch adds support for system-wide and runtime power management.
> > 
> > Ok.
> > 
> > > > Since System MMU is located in the same local power domain of its
> > > > master H/W, System MMU must be initialized before it is working if
> > > > its power domain was ever turned off. TLB invalidation according to
> > > > unmapping on page tables must also be performed while power domain
> > > > is
> > > > turned on.
> > > > 
> > > > This patch ensures that resume and runtime_resume(restore_state)
> > > > functions in this driver is called before the calls to resume and
> > > > runtime_resume callback functions in the drivers of master H/Ws.
> > > > Likewise, suspend and runtime_suspend(save_state) functions in this
> > > > driver is called after the calls to suspend and runtime_suspend in
> > > > the
> > > > drivers of master H/Ws.
> > > > 
> > > > In order to get benefit of this support, the master H/W and its
> > > > System
> > > > MMU must resides in the same power domain in terms of Linux kernel.
> > > > If
> > > > a master H/W does not use generic I/O power domain, its driver must
> > > > call iommu_attach_device() after its local power domain is turned
> > > > on,
> > > > iommu_detach_device before turned off.
> > > 
> > > I don't get the point of this last paragraph. What a power domain can
> > > be in other terms? Is there any other way to support power domains on
> > > Exynos than generic power domains?
> > 
> > I just addressed the case a device driver turns off local power of its
> > device without the help of generic I/O powerdomain.
> 
> Out of curiosity, do we have such cases for Exynos in mainline kernel? 
> IMHO this is what the generic PM core is for and drivers shouldn't care 
> about such low level PM details.

I don't know if there is the case and I also agree with you.

I hope that there is no case that I addressed.
I just mentioned an exceptional case.
The best way is that all device drivers of master H/W of System MMU register
their defvice in a generic i/o powerdomain.

Thank you.

KyongHo.
--
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 v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-11 Thread Cho KyongHo
On Fri, 09 Aug 2013 10:32:40 +0200, Tomasz Figa wrote:
 On Friday 09 of August 2013 16:49:43 Cho KyongHo wrote:
  On Fri, 09 Aug 2013 01:03:05 +0200, Tomasz Figa wrote:
   Hi KyongHo,
   
   nit: Please drop the trailing dot at the end of patch subject.
  
  Oh. I didn't catch that.
  Thank you.
  
   On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
This adds support for Advance Power Management and Runtime Power
Management.
   
   This patch adds support for system-wide and runtime power management.
  
  Ok.
  
Since System MMU is located in the same local power domain of its
master H/W, System MMU must be initialized before it is working if
its power domain was ever turned off. TLB invalidation according to
unmapping on page tables must also be performed while power domain
is
turned on.

This patch ensures that resume and runtime_resume(restore_state)
functions in this driver is called before the calls to resume and
runtime_resume callback functions in the drivers of master H/Ws.
Likewise, suspend and runtime_suspend(save_state) functions in this
driver is called after the calls to suspend and runtime_suspend in
the
drivers of master H/Ws.

In order to get benefit of this support, the master H/W and its
System
MMU must resides in the same power domain in terms of Linux kernel.
If
a master H/W does not use generic I/O power domain, its driver must
call iommu_attach_device() after its local power domain is turned
on,
iommu_detach_device before turned off.
   
   I don't get the point of this last paragraph. What a power domain can
   be in other terms? Is there any other way to support power domains on
   Exynos than generic power domains?
  
  I just addressed the case a device driver turns off local power of its
  device without the help of generic I/O powerdomain.
 
 Out of curiosity, do we have such cases for Exynos in mainline kernel? 
 IMHO this is what the generic PM core is for and drivers shouldn't care 
 about such low level PM details.

I don't know if there is the case and I also agree with you.

I hope that there is no case that I addressed.
I just mentioned an exceptional case.
The best way is that all device drivers of master H/W of System MMU register
their defvice in a generic i/o powerdomain.

Thank you.

KyongHo.
--
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 v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-09 Thread Tomasz Figa
On Friday 09 of August 2013 16:49:43 Cho KyongHo wrote:
> On Fri, 09 Aug 2013 01:03:05 +0200, Tomasz Figa wrote:
> > Hi KyongHo,
> > 
> > nit: Please drop the trailing dot at the end of patch subject.
> 
> Oh. I didn't catch that.
> Thank you.
> 
> > On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
> > > This adds support for Advance Power Management and Runtime Power
> > > Management.
> > 
> > This patch adds support for system-wide and runtime power management.
> 
> Ok.
> 
> > > Since System MMU is located in the same local power domain of its
> > > master H/W, System MMU must be initialized before it is working if
> > > its power domain was ever turned off. TLB invalidation according to
> > > unmapping on page tables must also be performed while power domain
> > > is
> > > turned on.
> > > 
> > > This patch ensures that resume and runtime_resume(restore_state)
> > > functions in this driver is called before the calls to resume and
> > > runtime_resume callback functions in the drivers of master H/Ws.
> > > Likewise, suspend and runtime_suspend(save_state) functions in this
> > > driver is called after the calls to suspend and runtime_suspend in
> > > the
> > > drivers of master H/Ws.
> > > 
> > > In order to get benefit of this support, the master H/W and its
> > > System
> > > MMU must resides in the same power domain in terms of Linux kernel.
> > > If
> > > a master H/W does not use generic I/O power domain, its driver must
> > > call iommu_attach_device() after its local power domain is turned
> > > on,
> > > iommu_detach_device before turned off.
> > 
> > I don't get the point of this last paragraph. What a power domain can
> > be in other terms? Is there any other way to support power domains on
> > Exynos than generic power domains?
> 
> I just addressed the case a device driver turns off local power of its
> device without the help of generic I/O powerdomain.

Out of curiosity, do we have such cases for Exynos in mainline kernel? 
IMHO this is what the generic PM core is for and drivers shouldn't care 
about such low level PM details.

> > > Signed-off-by: Cho KyongHo 
> > > ---
> > > 
> > >  drivers/iommu/exynos-iommu.c |  235
> > > 
> > > +- 1 files changed, 231
> > > insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/exynos-iommu.c
> > > b/drivers/iommu/exynos-iommu.c index 9e64483..56aead9 100644
> > > --- a/drivers/iommu/exynos-iommu.c
> > > +++ b/drivers/iommu/exynos-iommu.c
> > > @@ -28,6 +28,7 @@
> > > 
> > >  #include 
> > >  #include 
> > >  #include 
> > > 
> > > +#include 
> > > 
> > >  #include 
> > >  
> > >  #include 
> > > 
> > > @@ -186,6 +187,7 @@ struct sysmmu_drvdata {
> > > 
> > >   int activations;
> > >   rwlock_t lock;
> > >   struct iommu_domain *domain;
> > > 
> > > + bool runtime_active;
> > > 
> > >   unsigned long pgtable;
> > >   void __iomem *sfrbases[0];
> > >  
> > >  };
> > > 
> > > @@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct
> > > sysmmu_drvdata
> > > *data) data->pgtable = 0;
> > > 
> > >   data->domain = NULL;
> > > 
> > > - __sysmmu_disable_nocount(data);
> > > + if (data->runtime_active)
> > > + __sysmmu_disable_nocount(data);
> > > 
> > >   dev_dbg(data->sysmmu, "Disabled\n");
> > >   
> > >   } else  {
> > > 
> > > @@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata
> > > *data, data->pgtable = pgtable;
> > > 
> > >   data->domain = domain;
> > > 
> > > - __sysmmu_enable_nocount(data);
> > > + if (data->runtime_active)
> > > + __sysmmu_enable_nocount(data);
> > > 
> > >   dev_dbg(data->sysmmu, "Enabled\n");
> > >   
> > >   } else {
> > > 
> > > @@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct
> > > device *dev, unsigned long iova) data =
> > > dev_get_drvdata(client->sysmmu[i]);
> > > 
> > >   read_lock_irqsave(>lock, flags);
> > > 
> > > - if (is_sysmmu_active(data)) {
> > > + if (is_sysmmu_active(data) && data->runtime_active) {
> > > 
> > >   int i;
> > >   clk_enable(data->clk_master);
> > >   for (i = 0; i < data->nsfrs; i++)
> > > 
> > > @@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device
> > > *dev) data = dev_get_drvdata(client->sysmmu[i]);
> > > 
> > >   read_lock_irqsave(>lock, flags);
> > > 
> > > - if (is_sysmmu_active(data)) {
> > > + if (is_sysmmu_active(data) &&
> > > + data->runtime_active) {
> > 
> > nit: The whole if condition fits into single line.
> 
> Ok.
> 
> > >   int i;
> > >   for (i = 0; i < data->nsfrs; i++) {
> > >   
> > >   clk_enable(data->clk_master);
> > > 
> > > @@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct
> > > platform_device *pdev) }
> > > 
> > >   }
> > > 
> > > + 

Re: [PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-09 Thread Cho KyongHo
On Fri, 09 Aug 2013 01:03:05 +0200, Tomasz Figa wrote:
> Hi KyongHo,
> 
> nit: Please drop the trailing dot at the end of patch subject.
> 

Oh. I didn't catch that.
Thank you.

> On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
> > This adds support for Advance Power Management and Runtime Power
> > Management.
> 
> This patch adds support for system-wide and runtime power management.
> 

Ok.

> > Since System MMU is located in the same local power domain of its
> > master H/W, System MMU must be initialized before it is working if
> > its power domain was ever turned off. TLB invalidation according to
> > unmapping on page tables must also be performed while power domain is
> > turned on.
> > 
> > This patch ensures that resume and runtime_resume(restore_state)
> > functions in this driver is called before the calls to resume and
> > runtime_resume callback functions in the drivers of master H/Ws.
> > Likewise, suspend and runtime_suspend(save_state) functions in this
> > driver is called after the calls to suspend and runtime_suspend in the
> > drivers of master H/Ws.
> > 
> > In order to get benefit of this support, the master H/W and its System
> > MMU must resides in the same power domain in terms of Linux kernel. If
> > a master H/W does not use generic I/O power domain, its driver must
> > call iommu_attach_device() after its local power domain is turned on,
> > iommu_detach_device before turned off.
> 
> I don't get the point of this last paragraph. What a power domain can be 
> in other terms? Is there any other way to support power domains on Exynos 
> than generic power domains?
> 

I just addressed the case a device driver turns off local power of its
device without the help of generic I/O powerdomain.

> > Signed-off-by: Cho KyongHo 
> > ---
> >  drivers/iommu/exynos-iommu.c |  235
> > +- 1 files changed, 231
> > insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> > index 9e64483..56aead9 100644
> > --- a/drivers/iommu/exynos-iommu.c
> > +++ b/drivers/iommu/exynos-iommu.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> > 
> >  #include 
> > @@ -186,6 +187,7 @@ struct sysmmu_drvdata {
> > int activations;
> > rwlock_t lock;
> > struct iommu_domain *domain;
> > +   bool runtime_active;
> > unsigned long pgtable;
> > void __iomem *sfrbases[0];
> >  };
> > @@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata
> > *data) data->pgtable = 0;
> > data->domain = NULL;
> > 
> > -   __sysmmu_disable_nocount(data);
> > +   if (data->runtime_active)
> > +   __sysmmu_disable_nocount(data);
> > 
> > dev_dbg(data->sysmmu, "Disabled\n");
> > } else  {
> > @@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata
> > *data, data->pgtable = pgtable;
> > data->domain = domain;
> > 
> > -   __sysmmu_enable_nocount(data);
> > +   if (data->runtime_active)
> > +   __sysmmu_enable_nocount(data);
> > 
> > dev_dbg(data->sysmmu, "Enabled\n");
> > } else {
> > @@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct
> > device *dev, unsigned long iova) data =
> > dev_get_drvdata(client->sysmmu[i]);
> > 
> > read_lock_irqsave(>lock, flags);
> > -   if (is_sysmmu_active(data)) {
> > +   if (is_sysmmu_active(data) && data->runtime_active) {
> > int i;
> > clk_enable(data->clk_master);
> > for (i = 0; i < data->nsfrs; i++)
> > @@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device
> > *dev) data = dev_get_drvdata(client->sysmmu[i]);
> > 
> > read_lock_irqsave(>lock, flags);
> > -   if (is_sysmmu_active(data)) {
> > +   if (is_sysmmu_active(data) &&
> > +   data->runtime_active) {
> 
> nit: The whole if condition fits into single line.

Ok.

> 
> > int i;
> > for (i = 0; i < data->nsfrs; i++) {
> > clk_enable(data->clk_master);
> > @@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct
> > platform_device *pdev) }
> > }
> > 
> > +   pm_runtime_enable(dev);
> > +
> > data->sysmmu = dev;
> > 
> > data->clk = devm_clk_get(dev, "sysmmu");
> > @@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct
> > platform_device *pdev) return ret;
> > }
> > 
> > +   data->runtime_active = !pm_runtime_enabled(dev);
> > +
> > rwlock_init(>lock);
> > 
> > platform_set_drvdata(pdev, data);
> > @@ -744,6 +753,34 @@ static int __init exynos_sysmmu_probe(struct
> > platform_device *pdev) return ret;
> >  }
> > 
> > +#ifdef CONFIG_PM_SLEEP
> > +static int sysmmu_suspend(struct device *dev)
> > +{
> > +   struct sysmmu_drvdata *data = dev_get_drvdata(dev);

Re: [PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-09 Thread Cho KyongHo
On Fri, 09 Aug 2013 01:03:05 +0200, Tomasz Figa wrote:
 Hi KyongHo,
 
 nit: Please drop the trailing dot at the end of patch subject.
 

Oh. I didn't catch that.
Thank you.

 On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
  This adds support for Advance Power Management and Runtime Power
  Management.
 
 This patch adds support for system-wide and runtime power management.
 

Ok.

  Since System MMU is located in the same local power domain of its
  master H/W, System MMU must be initialized before it is working if
  its power domain was ever turned off. TLB invalidation according to
  unmapping on page tables must also be performed while power domain is
  turned on.
  
  This patch ensures that resume and runtime_resume(restore_state)
  functions in this driver is called before the calls to resume and
  runtime_resume callback functions in the drivers of master H/Ws.
  Likewise, suspend and runtime_suspend(save_state) functions in this
  driver is called after the calls to suspend and runtime_suspend in the
  drivers of master H/Ws.
  
  In order to get benefit of this support, the master H/W and its System
  MMU must resides in the same power domain in terms of Linux kernel. If
  a master H/W does not use generic I/O power domain, its driver must
  call iommu_attach_device() after its local power domain is turned on,
  iommu_detach_device before turned off.
 
 I don't get the point of this last paragraph. What a power domain can be 
 in other terms? Is there any other way to support power domains on Exynos 
 than generic power domains?
 

I just addressed the case a device driver turns off local power of its
device without the help of generic I/O powerdomain.

  Signed-off-by: Cho KyongHo pullip@samsung.com
  ---
   drivers/iommu/exynos-iommu.c |  235
  +- 1 files changed, 231
  insertions(+), 4 deletions(-)
  
  diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
  index 9e64483..56aead9 100644
  --- a/drivers/iommu/exynos-iommu.c
  +++ b/drivers/iommu/exynos-iommu.c
  @@ -28,6 +28,7 @@
   #include linux/export.h
   #include linux/of.h
   #include linux/of_platform.h
  +#include linux/pm_domain.h
   #include linux/notifier.h
  
   #include asm/cacheflush.h
  @@ -186,6 +187,7 @@ struct sysmmu_drvdata {
  int activations;
  rwlock_t lock;
  struct iommu_domain *domain;
  +   bool runtime_active;
  unsigned long pgtable;
  void __iomem *sfrbases[0];
   };
  @@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata
  *data) data-pgtable = 0;
  data-domain = NULL;
  
  -   __sysmmu_disable_nocount(data);
  +   if (data-runtime_active)
  +   __sysmmu_disable_nocount(data);
  
  dev_dbg(data-sysmmu, Disabled\n);
  } else  {
  @@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata
  *data, data-pgtable = pgtable;
  data-domain = domain;
  
  -   __sysmmu_enable_nocount(data);
  +   if (data-runtime_active)
  +   __sysmmu_enable_nocount(data);
  
  dev_dbg(data-sysmmu, Enabled\n);
  } else {
  @@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct
  device *dev, unsigned long iova) data =
  dev_get_drvdata(client-sysmmu[i]);
  
  read_lock_irqsave(data-lock, flags);
  -   if (is_sysmmu_active(data)) {
  +   if (is_sysmmu_active(data)  data-runtime_active) {
  int i;
  clk_enable(data-clk_master);
  for (i = 0; i  data-nsfrs; i++)
  @@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device
  *dev) data = dev_get_drvdata(client-sysmmu[i]);
  
  read_lock_irqsave(data-lock, flags);
  -   if (is_sysmmu_active(data)) {
  +   if (is_sysmmu_active(data) 
  +   data-runtime_active) {
 
 nit: The whole if condition fits into single line.

Ok.

 
  int i;
  for (i = 0; i  data-nsfrs; i++) {
  clk_enable(data-clk_master);
  @@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct
  platform_device *pdev) }
  }
  
  +   pm_runtime_enable(dev);
  +
  data-sysmmu = dev;
  
  data-clk = devm_clk_get(dev, sysmmu);
  @@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct
  platform_device *pdev) return ret;
  }
  
  +   data-runtime_active = !pm_runtime_enabled(dev);
  +
  rwlock_init(data-lock);
  
  platform_set_drvdata(pdev, data);
  @@ -744,6 +753,34 @@ static int __init exynos_sysmmu_probe(struct
  platform_device *pdev) return ret;
   }
  
  +#ifdef CONFIG_PM_SLEEP
  +static int sysmmu_suspend(struct device *dev)
  +{
  +   struct sysmmu_drvdata *data = dev_get_drvdata(dev);
  +   unsigned long flags;
  +   read_lock_irqsave(data-lock, flags);
  +   if (is_sysmmu_active(data) 
  +   (!pm_runtime_enabled(dev) 

Re: [PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-09 Thread Tomasz Figa
On Friday 09 of August 2013 16:49:43 Cho KyongHo wrote:
 On Fri, 09 Aug 2013 01:03:05 +0200, Tomasz Figa wrote:
  Hi KyongHo,
  
  nit: Please drop the trailing dot at the end of patch subject.
 
 Oh. I didn't catch that.
 Thank you.
 
  On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
   This adds support for Advance Power Management and Runtime Power
   Management.
  
  This patch adds support for system-wide and runtime power management.
 
 Ok.
 
   Since System MMU is located in the same local power domain of its
   master H/W, System MMU must be initialized before it is working if
   its power domain was ever turned off. TLB invalidation according to
   unmapping on page tables must also be performed while power domain
   is
   turned on.
   
   This patch ensures that resume and runtime_resume(restore_state)
   functions in this driver is called before the calls to resume and
   runtime_resume callback functions in the drivers of master H/Ws.
   Likewise, suspend and runtime_suspend(save_state) functions in this
   driver is called after the calls to suspend and runtime_suspend in
   the
   drivers of master H/Ws.
   
   In order to get benefit of this support, the master H/W and its
   System
   MMU must resides in the same power domain in terms of Linux kernel.
   If
   a master H/W does not use generic I/O power domain, its driver must
   call iommu_attach_device() after its local power domain is turned
   on,
   iommu_detach_device before turned off.
  
  I don't get the point of this last paragraph. What a power domain can
  be in other terms? Is there any other way to support power domains on
  Exynos than generic power domains?
 
 I just addressed the case a device driver turns off local power of its
 device without the help of generic I/O powerdomain.

Out of curiosity, do we have such cases for Exynos in mainline kernel? 
IMHO this is what the generic PM core is for and drivers shouldn't care 
about such low level PM details.

   Signed-off-by: Cho KyongHo pullip@samsung.com
   ---
   
drivers/iommu/exynos-iommu.c |  235
   
   +- 1 files changed, 231
   insertions(+), 4 deletions(-)
   
   diff --git a/drivers/iommu/exynos-iommu.c
   b/drivers/iommu/exynos-iommu.c index 9e64483..56aead9 100644
   --- a/drivers/iommu/exynos-iommu.c
   +++ b/drivers/iommu/exynos-iommu.c
   @@ -28,6 +28,7 @@
   
#include linux/export.h
#include linux/of.h
#include linux/of_platform.h
   
   +#include linux/pm_domain.h
   
#include linux/notifier.h

#include asm/cacheflush.h
   
   @@ -186,6 +187,7 @@ struct sysmmu_drvdata {
   
 int activations;
 rwlock_t lock;
 struct iommu_domain *domain;
   
   + bool runtime_active;
   
 unsigned long pgtable;
 void __iomem *sfrbases[0];

};
   
   @@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct
   sysmmu_drvdata
   *data) data-pgtable = 0;
   
 data-domain = NULL;
   
   - __sysmmu_disable_nocount(data);
   + if (data-runtime_active)
   + __sysmmu_disable_nocount(data);
   
 dev_dbg(data-sysmmu, Disabled\n);
 
 } else  {
   
   @@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata
   *data, data-pgtable = pgtable;
   
 data-domain = domain;
   
   - __sysmmu_enable_nocount(data);
   + if (data-runtime_active)
   + __sysmmu_enable_nocount(data);
   
 dev_dbg(data-sysmmu, Enabled\n);
 
 } else {
   
   @@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct
   device *dev, unsigned long iova) data =
   dev_get_drvdata(client-sysmmu[i]);
   
 read_lock_irqsave(data-lock, flags);
   
   - if (is_sysmmu_active(data)) {
   + if (is_sysmmu_active(data)  data-runtime_active) {
   
 int i;
 clk_enable(data-clk_master);
 for (i = 0; i  data-nsfrs; i++)
   
   @@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device
   *dev) data = dev_get_drvdata(client-sysmmu[i]);
   
 read_lock_irqsave(data-lock, flags);
   
   - if (is_sysmmu_active(data)) {
   + if (is_sysmmu_active(data) 
   + data-runtime_active) {
  
  nit: The whole if condition fits into single line.
 
 Ok.
 
 int i;
 for (i = 0; i  data-nsfrs; i++) {
 
 clk_enable(data-clk_master);
   
   @@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct
   platform_device *pdev) }
   
 }
   
   + pm_runtime_enable(dev);
   +
   
 data-sysmmu = dev;
 
 data-clk = devm_clk_get(dev, sysmmu);
   
   @@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct
   platform_device *pdev) return ret;
   
 }
   
   + data-runtime_active = !pm_runtime_enabled(dev);
   +
   
 rwlock_init(data-lock);
 

Re: [PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-08 Thread Tomasz Figa
Hi KyongHo,

nit: Please drop the trailing dot at the end of patch subject.

On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
> This adds support for Advance Power Management and Runtime Power
> Management.

This patch adds support for system-wide and runtime power management.

> Since System MMU is located in the same local power domain of its
> master H/W, System MMU must be initialized before it is working if
> its power domain was ever turned off. TLB invalidation according to
> unmapping on page tables must also be performed while power domain is
> turned on.
> 
> This patch ensures that resume and runtime_resume(restore_state)
> functions in this driver is called before the calls to resume and
> runtime_resume callback functions in the drivers of master H/Ws.
> Likewise, suspend and runtime_suspend(save_state) functions in this
> driver is called after the calls to suspend and runtime_suspend in the
> drivers of master H/Ws.
> 
> In order to get benefit of this support, the master H/W and its System
> MMU must resides in the same power domain in terms of Linux kernel. If
> a master H/W does not use generic I/O power domain, its driver must
> call iommu_attach_device() after its local power domain is turned on,
> iommu_detach_device before turned off.

I don't get the point of this last paragraph. What a power domain can be 
in other terms? Is there any other way to support power domains on Exynos 
than generic power domains?

> Signed-off-by: Cho KyongHo 
> ---
>  drivers/iommu/exynos-iommu.c |  235
> +- 1 files changed, 231
> insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 9e64483..56aead9 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> 
>  #include 
> @@ -186,6 +187,7 @@ struct sysmmu_drvdata {
>   int activations;
>   rwlock_t lock;
>   struct iommu_domain *domain;
> + bool runtime_active;
>   unsigned long pgtable;
>   void __iomem *sfrbases[0];
>  };
> @@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata
> *data) data->pgtable = 0;
>   data->domain = NULL;
> 
> - __sysmmu_disable_nocount(data);
> + if (data->runtime_active)
> + __sysmmu_disable_nocount(data);
> 
>   dev_dbg(data->sysmmu, "Disabled\n");
>   } else  {
> @@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata
> *data, data->pgtable = pgtable;
>   data->domain = domain;
> 
> - __sysmmu_enable_nocount(data);
> + if (data->runtime_active)
> + __sysmmu_enable_nocount(data);
> 
>   dev_dbg(data->sysmmu, "Enabled\n");
>   } else {
> @@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct
> device *dev, unsigned long iova) data =
> dev_get_drvdata(client->sysmmu[i]);
> 
>   read_lock_irqsave(>lock, flags);
> - if (is_sysmmu_active(data)) {
> + if (is_sysmmu_active(data) && data->runtime_active) {
>   int i;
>   clk_enable(data->clk_master);
>   for (i = 0; i < data->nsfrs; i++)
> @@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device
> *dev) data = dev_get_drvdata(client->sysmmu[i]);
> 
>   read_lock_irqsave(>lock, flags);
> - if (is_sysmmu_active(data)) {
> + if (is_sysmmu_active(data) &&
> + data->runtime_active) {

nit: The whole if condition fits into single line.

>   int i;
>   for (i = 0; i < data->nsfrs; i++) {
>   clk_enable(data->clk_master);
> @@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct
> platform_device *pdev) }
>   }
> 
> + pm_runtime_enable(dev);
> +
>   data->sysmmu = dev;
> 
>   data->clk = devm_clk_get(dev, "sysmmu");
> @@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct
> platform_device *pdev) return ret;
>   }
> 
> + data->runtime_active = !pm_runtime_enabled(dev);
> +
>   rwlock_init(>lock);
> 
>   platform_set_drvdata(pdev, data);
> @@ -744,6 +753,34 @@ static int __init exynos_sysmmu_probe(struct
> platform_device *pdev) return ret;
>  }
> 
> +#ifdef CONFIG_PM_SLEEP
> +static int sysmmu_suspend(struct device *dev)
> +{
> + struct sysmmu_drvdata *data = dev_get_drvdata(dev);
> + unsigned long flags;
> + read_lock_irqsave(>lock, flags);
> + if (is_sysmmu_active(data) &&
> + (!pm_runtime_enabled(dev) || data->runtime_active))
> + __sysmmu_disable_nocount(data);
> + read_unlock_irqrestore(>lock, flags);
> + return 0;
> +}
> +
> +static int sysmmu_resume(struct device *dev)
> +{
> + struct sysmmu_drvdata *data = 

[PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-08 Thread Cho KyongHo
This adds support for Advance Power Management and Runtime Power
Management.

Since System MMU is located in the same local power domain of its
master H/W, System MMU must be initialized before it is working if
its power domain was ever turned off. TLB invalidation according to
unmapping on page tables must also be performed while power domain is
turned on.

This patch ensures that resume and runtime_resume(restore_state)
functions in this driver is called before the calls to resume and
runtime_resume callback functions in the drivers of master H/Ws.
Likewise, suspend and runtime_suspend(save_state) functions in this
driver is called after the calls to suspend and runtime_suspend in the
drivers of master H/Ws.

In order to get benefit of this support, the master H/W and its System
MMU must resides in the same power domain in terms of Linux kernel. If
a master H/W does not use generic I/O power domain, its driver must
call iommu_attach_device() after its local power domain is turned on,
iommu_detach_device before turned off.

Signed-off-by: Cho KyongHo 
---
 drivers/iommu/exynos-iommu.c |  235 +-
 1 files changed, 231 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 9e64483..56aead9 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -186,6 +187,7 @@ struct sysmmu_drvdata {
int activations;
rwlock_t lock;
struct iommu_domain *domain;
+   bool runtime_active;
unsigned long pgtable;
void __iomem *sfrbases[0];
 };
@@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata *data)
data->pgtable = 0;
data->domain = NULL;
 
-   __sysmmu_disable_nocount(data);
+   if (data->runtime_active)
+   __sysmmu_disable_nocount(data);
 
dev_dbg(data->sysmmu, "Disabled\n");
} else  {
@@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata *data,
data->pgtable = pgtable;
data->domain = domain;
 
-   __sysmmu_enable_nocount(data);
+   if (data->runtime_active)
+   __sysmmu_enable_nocount(data);
 
dev_dbg(data->sysmmu, "Enabled\n");
} else {
@@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct device *dev, 
unsigned long iova)
data = dev_get_drvdata(client->sysmmu[i]);
 
read_lock_irqsave(>lock, flags);
-   if (is_sysmmu_active(data)) {
+   if (is_sysmmu_active(data) && data->runtime_active) {
int i;
clk_enable(data->clk_master);
for (i = 0; i < data->nsfrs; i++)
@@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
data = dev_get_drvdata(client->sysmmu[i]);
 
read_lock_irqsave(>lock, flags);
-   if (is_sysmmu_active(data)) {
+   if (is_sysmmu_active(data) &&
+   data->runtime_active) {
int i;
for (i = 0; i < data->nsfrs; i++) {
clk_enable(data->clk_master);
@@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct 
platform_device *pdev)
}
}
 
+   pm_runtime_enable(dev);
+
data->sysmmu = dev;
 
data->clk = devm_clk_get(dev, "sysmmu");
@@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct 
platform_device *pdev)
return ret;
}
 
+   data->runtime_active = !pm_runtime_enabled(dev);
+
rwlock_init(>lock);
 
platform_set_drvdata(pdev, data);
@@ -744,6 +753,34 @@ static int __init exynos_sysmmu_probe(struct 
platform_device *pdev)
return ret;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int sysmmu_suspend(struct device *dev)
+{
+   struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+   unsigned long flags;
+   read_lock_irqsave(>lock, flags);
+   if (is_sysmmu_active(data) &&
+   (!pm_runtime_enabled(dev) || data->runtime_active))
+   __sysmmu_disable_nocount(data);
+   read_unlock_irqrestore(>lock, flags);
+   return 0;
+}
+
+static int sysmmu_resume(struct device *dev)
+{
+   struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+   unsigned long flags;
+   read_lock_irqsave(>lock, flags);
+   if (is_sysmmu_active(data) &&
+   (!pm_runtime_enabled(dev) || data->runtime_active))
+   __sysmmu_enable_nocount(data);
+   read_unlock_irqrestore(>lock, flags);
+   return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(sysmmu_pm_ops, sysmmu_suspend, sysmmu_resume);
+
 #ifdef CONFIG_OF
 static struct of_device_id sysmmu_of_match[] __initconst = {

[PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-08 Thread Cho KyongHo
This adds support for Advance Power Management and Runtime Power
Management.

Since System MMU is located in the same local power domain of its
master H/W, System MMU must be initialized before it is working if
its power domain was ever turned off. TLB invalidation according to
unmapping on page tables must also be performed while power domain is
turned on.

This patch ensures that resume and runtime_resume(restore_state)
functions in this driver is called before the calls to resume and
runtime_resume callback functions in the drivers of master H/Ws.
Likewise, suspend and runtime_suspend(save_state) functions in this
driver is called after the calls to suspend and runtime_suspend in the
drivers of master H/Ws.

In order to get benefit of this support, the master H/W and its System
MMU must resides in the same power domain in terms of Linux kernel. If
a master H/W does not use generic I/O power domain, its driver must
call iommu_attach_device() after its local power domain is turned on,
iommu_detach_device before turned off.

Signed-off-by: Cho KyongHo pullip@samsung.com
---
 drivers/iommu/exynos-iommu.c |  235 +-
 1 files changed, 231 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 9e64483..56aead9 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -28,6 +28,7 @@
 #include linux/export.h
 #include linux/of.h
 #include linux/of_platform.h
+#include linux/pm_domain.h
 #include linux/notifier.h
 
 #include asm/cacheflush.h
@@ -186,6 +187,7 @@ struct sysmmu_drvdata {
int activations;
rwlock_t lock;
struct iommu_domain *domain;
+   bool runtime_active;
unsigned long pgtable;
void __iomem *sfrbases[0];
 };
@@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata *data)
data-pgtable = 0;
data-domain = NULL;
 
-   __sysmmu_disable_nocount(data);
+   if (data-runtime_active)
+   __sysmmu_disable_nocount(data);
 
dev_dbg(data-sysmmu, Disabled\n);
} else  {
@@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata *data,
data-pgtable = pgtable;
data-domain = domain;
 
-   __sysmmu_enable_nocount(data);
+   if (data-runtime_active)
+   __sysmmu_enable_nocount(data);
 
dev_dbg(data-sysmmu, Enabled\n);
} else {
@@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct device *dev, 
unsigned long iova)
data = dev_get_drvdata(client-sysmmu[i]);
 
read_lock_irqsave(data-lock, flags);
-   if (is_sysmmu_active(data)) {
+   if (is_sysmmu_active(data)  data-runtime_active) {
int i;
clk_enable(data-clk_master);
for (i = 0; i  data-nsfrs; i++)
@@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device *dev)
data = dev_get_drvdata(client-sysmmu[i]);
 
read_lock_irqsave(data-lock, flags);
-   if (is_sysmmu_active(data)) {
+   if (is_sysmmu_active(data) 
+   data-runtime_active) {
int i;
for (i = 0; i  data-nsfrs; i++) {
clk_enable(data-clk_master);
@@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct 
platform_device *pdev)
}
}
 
+   pm_runtime_enable(dev);
+
data-sysmmu = dev;
 
data-clk = devm_clk_get(dev, sysmmu);
@@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct 
platform_device *pdev)
return ret;
}
 
+   data-runtime_active = !pm_runtime_enabled(dev);
+
rwlock_init(data-lock);
 
platform_set_drvdata(pdev, data);
@@ -744,6 +753,34 @@ static int __init exynos_sysmmu_probe(struct 
platform_device *pdev)
return ret;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int sysmmu_suspend(struct device *dev)
+{
+   struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+   unsigned long flags;
+   read_lock_irqsave(data-lock, flags);
+   if (is_sysmmu_active(data) 
+   (!pm_runtime_enabled(dev) || data-runtime_active))
+   __sysmmu_disable_nocount(data);
+   read_unlock_irqrestore(data-lock, flags);
+   return 0;
+}
+
+static int sysmmu_resume(struct device *dev)
+{
+   struct sysmmu_drvdata *data = dev_get_drvdata(dev);
+   unsigned long flags;
+   read_lock_irqsave(data-lock, flags);
+   if (is_sysmmu_active(data) 
+   (!pm_runtime_enabled(dev) || data-runtime_active))
+   __sysmmu_enable_nocount(data);
+   read_unlock_irqrestore(data-lock, flags);
+   return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(sysmmu_pm_ops, sysmmu_suspend, 

Re: [PATCH v9 14/16] iommu/exynos: add support for power management subsystems.

2013-08-08 Thread Tomasz Figa
Hi KyongHo,

nit: Please drop the trailing dot at the end of patch subject.

On Thursday 08 of August 2013 18:41:17 Cho KyongHo wrote:
 This adds support for Advance Power Management and Runtime Power
 Management.

This patch adds support for system-wide and runtime power management.

 Since System MMU is located in the same local power domain of its
 master H/W, System MMU must be initialized before it is working if
 its power domain was ever turned off. TLB invalidation according to
 unmapping on page tables must also be performed while power domain is
 turned on.
 
 This patch ensures that resume and runtime_resume(restore_state)
 functions in this driver is called before the calls to resume and
 runtime_resume callback functions in the drivers of master H/Ws.
 Likewise, suspend and runtime_suspend(save_state) functions in this
 driver is called after the calls to suspend and runtime_suspend in the
 drivers of master H/Ws.
 
 In order to get benefit of this support, the master H/W and its System
 MMU must resides in the same power domain in terms of Linux kernel. If
 a master H/W does not use generic I/O power domain, its driver must
 call iommu_attach_device() after its local power domain is turned on,
 iommu_detach_device before turned off.

I don't get the point of this last paragraph. What a power domain can be 
in other terms? Is there any other way to support power domains on Exynos 
than generic power domains?

 Signed-off-by: Cho KyongHo pullip@samsung.com
 ---
  drivers/iommu/exynos-iommu.c |  235
 +- 1 files changed, 231
 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
 index 9e64483..56aead9 100644
 --- a/drivers/iommu/exynos-iommu.c
 +++ b/drivers/iommu/exynos-iommu.c
 @@ -28,6 +28,7 @@
  #include linux/export.h
  #include linux/of.h
  #include linux/of_platform.h
 +#include linux/pm_domain.h
  #include linux/notifier.h
 
  #include asm/cacheflush.h
 @@ -186,6 +187,7 @@ struct sysmmu_drvdata {
   int activations;
   rwlock_t lock;
   struct iommu_domain *domain;
 + bool runtime_active;
   unsigned long pgtable;
   void __iomem *sfrbases[0];
  };
 @@ -438,7 +440,8 @@ static bool __sysmmu_disable(struct sysmmu_drvdata
 *data) data-pgtable = 0;
   data-domain = NULL;
 
 - __sysmmu_disable_nocount(data);
 + if (data-runtime_active)
 + __sysmmu_disable_nocount(data);
 
   dev_dbg(data-sysmmu, Disabled\n);
   } else  {
 @@ -505,7 +508,8 @@ static int __sysmmu_enable(struct sysmmu_drvdata
 *data, data-pgtable = pgtable;
   data-domain = domain;
 
 - __sysmmu_enable_nocount(data);
 + if (data-runtime_active)
 + __sysmmu_enable_nocount(data);
 
   dev_dbg(data-sysmmu, Enabled\n);
   } else {
 @@ -609,7 +613,7 @@ static void sysmmu_tlb_invalidate_entry(struct
 device *dev, unsigned long iova) data =
 dev_get_drvdata(client-sysmmu[i]);
 
   read_lock_irqsave(data-lock, flags);
 - if (is_sysmmu_active(data)) {
 + if (is_sysmmu_active(data)  data-runtime_active) {
   int i;
   clk_enable(data-clk_master);
   for (i = 0; i  data-nsfrs; i++)
 @@ -637,7 +641,8 @@ void exynos_sysmmu_tlb_invalidate(struct device
 *dev) data = dev_get_drvdata(client-sysmmu[i]);
 
   read_lock_irqsave(data-lock, flags);
 - if (is_sysmmu_active(data)) {
 + if (is_sysmmu_active(data) 
 + data-runtime_active) {

nit: The whole if condition fits into single line.

   int i;
   for (i = 0; i  data-nsfrs; i++) {
   clk_enable(data-clk_master);
 @@ -711,6 +716,8 @@ static int __init exynos_sysmmu_probe(struct
 platform_device *pdev) }
   }
 
 + pm_runtime_enable(dev);
 +
   data-sysmmu = dev;
 
   data-clk = devm_clk_get(dev, sysmmu);
 @@ -736,6 +743,8 @@ static int __init exynos_sysmmu_probe(struct
 platform_device *pdev) return ret;
   }
 
 + data-runtime_active = !pm_runtime_enabled(dev);
 +
   rwlock_init(data-lock);
 
   platform_set_drvdata(pdev, data);
 @@ -744,6 +753,34 @@ static int __init exynos_sysmmu_probe(struct
 platform_device *pdev) return ret;
  }
 
 +#ifdef CONFIG_PM_SLEEP
 +static int sysmmu_suspend(struct device *dev)
 +{
 + struct sysmmu_drvdata *data = dev_get_drvdata(dev);
 + unsigned long flags;
 + read_lock_irqsave(data-lock, flags);
 + if (is_sysmmu_active(data) 
 + (!pm_runtime_enabled(dev) || data-runtime_active))
 + __sysmmu_disable_nocount(data);
 + read_unlock_irqrestore(data-lock, flags);
 + return 0;
 +}
 +
 +static int sysmmu_resume(struct device *dev)
 +{
 + struct sysmmu_drvdata *data = dev_get_drvdata(dev);
 + unsigned