Re: [PATCH 3/3] ARM: EXYNOS: pm_domain: Bind devices to power domains using DT

2012-09-08 Thread Thomas Abraham 
On 6 September 2012 15:08, Tomasz Figa t.f...@samsung.com wrote:
 This patch adds a way to specify bindings between devices and power
 domains using device tree.

 A device can be bound to particular power domain by adding a
 power-domain property containing a phandle to the domain. The device
 will be bound to the domain before binding a driver to it and unbound
 after unbinding a driver from it.

 Signed-off-by: Tomasz Figa t.f...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  .../bindings/arm/exynos/power_domain.txt   | 13 +++-
  arch/arm/mach-exynos/pm_domains.c  | 82 
 ++
  2 files changed, 94 insertions(+), 1 deletion(-)

 diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt 
 b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
 index 843b546..8ed914f 100644
 --- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
 +++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
 @@ -4,14 +4,25 @@ Exynos processors include support for multiple power 
 domains which are used
  to gate power to one or more peripherals on the processor.

  Required Properties:
 -- compatiable: should be one of the following.
 +- compatible: should be one of the following.
  * samsung,exynos4210-pd - for exynos4210 type power domain.
  - reg: physical base address of the controller and length of memory mapped
  region.

 +Node of a device using power domains must have a power-domain property 
 defined
 +with a phandle to respective power domain.
 +
  Example:

 lcd0: power-domain-lcd0 {
 compatible = samsung,exynos4210-pd;
 reg = 0x10023C00 0x10;
 };
 +
 +Example of the node using power domain:
 +
 +   node {
 +   /* ... */
 +   power-domain = lcd0;
 +   /* ... */
 +   };

Since the value of power-domain property is mostly samsung specific,
should this be samsung,power-domain ?

 diff --git a/arch/arm/mach-exynos/pm_domains.c 
 b/arch/arm/mach-exynos/pm_domains.c
 index 5b7ce7e..7b3b8a3 100644
 --- a/arch/arm/mach-exynos/pm_domains.c
 +++ b/arch/arm/mach-exynos/pm_domains.c
 @@ -19,6 +19,8 @@
  #include linux/pm_domain.h
  #include linux/delay.h
  #include linux/of_address.h
 +#include linux/of_platform.h
 +#include linux/sched.h

  #include mach/regs-pmu.h
  #include plat/devs.h
 @@ -83,14 +85,89 @@ static struct exynos_pm_domain PD = { 
   \
  }

  #ifdef CONFIG_OF
 +static void exynos_add_device_to_domain(struct exynos_pm_domain *pd,
 +   struct device *dev)
 +{
 +   int ret;
 +
 +   dev_dbg(dev, adding to power domain %s\n, pd-pd.name);
 +
 +   while(1) {
 +   ret = pm_genpd_add_device(pd-pd, dev);
 +   if (ret != -EAGAIN)
 +   break;
 +   cond_resched();
 +   }
 +
 +   pm_genpd_dev_need_restore(dev, true);
 +}
 +
 +static void exynos_remove_device_from_domain(struct device *dev)
 +{
 +   struct generic_pm_domain *genpd = dev_to_genpd(dev);
 +   int ret;
 +
 +   dev_dbg(dev, removing from power domain %s\n, genpd-name);
 +
 +   while(1) {
 +   ret = pm_genpd_remove_device(genpd, dev);
 +   if (ret != -EAGAIN)
 +   break;
 +   cond_resched();
 +   }
 +}
 +
 +static void exynos_read_domain_from_dt(struct device *dev)
 +{
 +   struct platform_device *pd_pdev;
 +   struct exynos_pm_domain *pd;
 +   struct device_node *node;
 +
 +   node = of_parse_phandle(dev-of_node, power-domain, 0);
 +   if (!node)
 +   return;
 +   pd_pdev = of_find_device_by_node(node);
 +   if (!pd_pdev)
 +   return;
 +   pd = platform_get_drvdata(pd_pdev);
 +   exynos_add_device_to_domain(pd, dev);
 +}

The function exynos_read_domain_from_dt does more than reading the
domain from dt. It associates a device with a power domain. So should
it be renamed accordingly?

 +
 +static int exynos_pm_notifier_call(struct notifier_block *nb,
 +   unsigned long event, void *data)
 +{
 +   struct device *dev = data;
 +
 +   switch (event) {
 +   case BUS_NOTIFY_BIND_DRIVER:
 +   if (dev-of_node)
 +   exynos_read_domain_from_dt(dev);
 +
 +   break;
 +
 +   case BUS_NOTIFY_UNBOUND_DRIVER:
 +   exynos_remove_device_from_domain(dev);
 +
 +   break;
 +   }
 +   return NOTIFY_DONE;
 +}
 +
 +static struct notifier_block platform_nb = {
 +   .notifier_call = exynos_pm_notifier_call,
 +};

All the functions above are so generic (or can be made generic with
minor modifications) that it can be placed outside of mach-exynos. Or
better still, reusable for all platforms.

 +
  static __init int exynos_pm_dt_parse_domains(void)
  {
 +   struct

Re: [PATCH 3/3] ARM: EXYNOS: pm_domain: Bind devices to power domains using DT

2012-09-08 Thread Tomasz Figa 
Hi Thomas,

On Saturday 08 of September 2012 13:48:24 Thomas Abraham wrote:
  +Example of the node using power domain:
  +
  +   node {
  +   /* ... */
  +   power-domain = lcd0;
  +   /* ... */
  +   };
 
 Since the value of power-domain property is mostly samsung specific,
 should this be samsung,power-domain ?

Is there a convention of naming that defines such scheme? I have seen 
platform-specific properties without a prefix indicating the platform.

  +static void exynos_read_domain_from_dt(struct device *dev)
  +{
  +   struct platform_device *pd_pdev;
  +   struct exynos_pm_domain *pd;
  +   struct device_node *node;
  +
  +   node = of_parse_phandle(dev-of_node, power-domain, 0);
  +   if (!node)
  +   return;
  +   pd_pdev = of_find_device_by_node(node);
  +   if (!pd_pdev)
  +   return;
  +   pd = platform_get_drvdata(pd_pdev);
  +   exynos_add_device_to_domain(pd, dev);
  +}
 
 The function exynos_read_domain_from_dt does more than reading the
 domain from dt. It associates a device with a power domain. So should
 it be renamed accordingly?

Hmm, do you have an idea for a better name? I'm not good at inventing 
names.

  +
  +static int exynos_pm_notifier_call(struct notifier_block *nb,
  +   unsigned long event, void *data)
  +{
  +   struct device *dev = data;
  +
  +   switch (event) {
  +   case BUS_NOTIFY_BIND_DRIVER:
  +   if (dev-of_node)
  +   exynos_read_domain_from_dt(dev);
  +
  +   break;
  +
  +   case BUS_NOTIFY_UNBOUND_DRIVER:
  +   exynos_remove_device_from_domain(dev);
  +
  +   break;
  +   }
  +   return NOTIFY_DONE;
  +}
  +
  +static struct notifier_block platform_nb = {
  +   .notifier_call = exynos_pm_notifier_call,
  +};
 
 All the functions above are so generic (or can be made generic with
 minor modifications) that it can be placed outside of mach-exynos. Or
 better still, reusable for all platforms.

Right, I have considered this and even CC'ed Rafael with this patchset, but 
I forgot to mention about it in patch description.

Maybe I should send a separate RFC with a generic variant?

  
  --
  1.7.12
 
 This patch looks so nice. I learned a thing or two from this patch.
 Reviewed-by: Thomas Abraham thomas.abra...@linaro.org

Thanks ;)

--
Best regards,
Tomasz Figa

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

[PATCH 3/3] ARM: EXYNOS: pm_domain: Bind devices to power domains using DT

2012-09-06 Thread Tomasz Figa 
This patch adds a way to specify bindings between devices and power
domains using device tree.

A device can be bound to particular power domain by adding a
power-domain property containing a phandle to the domain. The device
will be bound to the domain before binding a driver to it and unbound
after unbinding a driver from it.

Signed-off-by: Tomasz Figa t.f...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
 .../bindings/arm/exynos/power_domain.txt   | 13 +++-
 arch/arm/mach-exynos/pm_domains.c  | 82 ++
 2 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt 
b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
index 843b546..8ed914f 100644
--- a/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
+++ b/Documentation/devicetree/bindings/arm/exynos/power_domain.txt
@@ -4,14 +4,25 @@ Exynos processors include support for multiple power domains 
which are used
 to gate power to one or more peripherals on the processor.
 
 Required Properties:
-- compatiable: should be one of the following.
+- compatible: should be one of the following.
 * samsung,exynos4210-pd - for exynos4210 type power domain.
 - reg: physical base address of the controller and length of memory mapped
 region.
 
+Node of a device using power domains must have a power-domain property defined
+with a phandle to respective power domain.
+
 Example:
 
lcd0: power-domain-lcd0 {
compatible = samsung,exynos4210-pd;
reg = 0x10023C00 0x10;
};
+
+Example of the node using power domain:
+
+   node {
+   /* ... */
+   power-domain = lcd0;
+   /* ... */
+   };
diff --git a/arch/arm/mach-exynos/pm_domains.c 
b/arch/arm/mach-exynos/pm_domains.c
index 5b7ce7e..7b3b8a3 100644
--- a/arch/arm/mach-exynos/pm_domains.c
+++ b/arch/arm/mach-exynos/pm_domains.c
@@ -19,6 +19,8 @@
 #include linux/pm_domain.h
 #include linux/delay.h
 #include linux/of_address.h
+#include linux/of_platform.h
+#include linux/sched.h
 
 #include mach/regs-pmu.h
 #include plat/devs.h
@@ -83,14 +85,89 @@ static struct exynos_pm_domain PD = {   
\
 }
 
 #ifdef CONFIG_OF
+static void exynos_add_device_to_domain(struct exynos_pm_domain *pd,
+   struct device *dev)
+{
+   int ret;
+
+   dev_dbg(dev, adding to power domain %s\n, pd-pd.name);
+
+   while(1) {
+   ret = pm_genpd_add_device(pd-pd, dev);
+   if (ret != -EAGAIN)
+   break;
+   cond_resched();
+   }
+
+   pm_genpd_dev_need_restore(dev, true);
+}
+
+static void exynos_remove_device_from_domain(struct device *dev)
+{
+   struct generic_pm_domain *genpd = dev_to_genpd(dev);
+   int ret;
+
+   dev_dbg(dev, removing from power domain %s\n, genpd-name);
+
+   while(1) {
+   ret = pm_genpd_remove_device(genpd, dev);
+   if (ret != -EAGAIN)
+   break;
+   cond_resched();
+   }
+}
+
+static void exynos_read_domain_from_dt(struct device *dev)
+{
+   struct platform_device *pd_pdev;
+   struct exynos_pm_domain *pd;
+   struct device_node *node;
+
+   node = of_parse_phandle(dev-of_node, power-domain, 0);
+   if (!node)
+   return;
+   pd_pdev = of_find_device_by_node(node);
+   if (!pd_pdev)
+   return;
+   pd = platform_get_drvdata(pd_pdev);
+   exynos_add_device_to_domain(pd, dev);
+}
+
+static int exynos_pm_notifier_call(struct notifier_block *nb,
+   unsigned long event, void *data)
+{
+   struct device *dev = data;
+
+   switch (event) {
+   case BUS_NOTIFY_BIND_DRIVER:
+   if (dev-of_node)
+   exynos_read_domain_from_dt(dev);
+
+   break;
+
+   case BUS_NOTIFY_UNBOUND_DRIVER:
+   exynos_remove_device_from_domain(dev);
+
+   break;
+   }
+   return NOTIFY_DONE;
+}
+
+static struct notifier_block platform_nb = {
+   .notifier_call = exynos_pm_notifier_call,
+};
+
 static __init int exynos_pm_dt_parse_domains(void)
 {
+   struct platform_device *pdev;
struct device_node *np;
 
for_each_compatible_node(np, NULL, samsung,exynos4210-pd) {
struct exynos_pm_domain *pd;
int on;
 
+   pdev = of_find_device_by_node(np);
+
pd = kzalloc(sizeof(*pd), GFP_KERNEL);
if (!pd) {
pr_err(%s: failed to allocate memory for domain\n,
@@ -105,10 +182,15 @@ static __init int exynos_pm_dt_parse_domains(void)
pd-pd.power_on = exynos_pd_power_on;
pd-pd.of_node = np;
 
+   platform_set_drvdata(pdev, pd);
+
on = __raw_readl(pd-base