Re: [PATCH 3/3] ARM: EXYNOS: pm_domain: Bind devices to power domains using DT
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
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
Re: [PATCH 3/3] ARM: EXYNOS: pm_domain: Bind devices to power domains using DT
On 6 September 2012 15:08, Tomasz Figa 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
> Signed-off-by: Kyungmin Park
> ---
> .../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
> #include
> #include
> +#include
> +#include
>
> #include
> #include
> @@ -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 platfo
[PATCH 3/3] ARM: EXYNOS: pm_domain: Bind devices to power domains using DT
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
Signed-off-by: Kyungmin Park
---
.../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
#include
#include
+#include
+#include
#include
#include
@@ -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 + 0x4) & S5P_INT_LOCAL_PWR_EN;
pm_genpd_init(&pd->pd, NULL, !on);
}
+
+ bus_register_notifi
