ARM: PM / Domains: Generic PM domains for CPUs/Clusters

In preparation for supporting IRQ-safe domains, allocate domain data outside the domain locks. These functions are not called in an atomic context, so we can always allocate memory using GFP_KERNEL. By allocating memory before the locks, we can safely lock the domain using spinlocks instead of mutexes.

Cc Ulf Hansson <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Krzysztof Koz?owski <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/base/power/domain.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

drivers/base/power/domain.c +15 -15

index 7666a1c..5fd1306 100644
@@ -1382,13 +1382,17 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 			   struct generic_pm_domain *subdomain)
 {
-	struct gpd_link *link;
+	struct gpd_link *link, *itr;
 	int ret = 0;
 
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(subdomain)
 	    || genpd == subdomain)
 		return -EINVAL;
 
+	link = kzalloc(sizeof(*link), GFP_KERNEL);
+	if (!link)
+		return -ENOMEM;
+
 	mutex_lock(&genpd->lock);
 	mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);
 

@@ -1398,18 +1402,13 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 		goto out;
 	}
 
-	list_for_each_entry(link, &genpd->master_links, master_node) {
-		if (link->slave == subdomain && link->master == genpd) {
+	list_for_each_entry(itr, &genpd->master_links, master_node) {
+		if (itr->slave == subdomain && itr->master == genpd) {
 			ret = -EINVAL;
 			goto out;
 		}
 	}
 
-	link = kzalloc(sizeof(*link), GFP_KERNEL);
-	if (!link) {
-		ret = -ENOMEM;
-		goto out;
-	}
 	link->master = genpd;
 	list_add_tail(&link->master_node, &genpd->master_links);
 	link->slave = subdomain;

@@ -1420,7 +1419,8 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
  out:
 	mutex_unlock(&subdomain->lock);
 	mutex_unlock(&genpd->lock);
-
+	if (ret)
+		kfree(link);
 	return ret;
 }
 

@@ -1511,17 +1511,17 @@ int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state)
 	if (IS_ERR_OR_NULL(genpd) || state < 0)
 		return -EINVAL;
 
+	cpuidle_data = kzalloc(sizeof(*cpuidle_data), GFP_KERNEL);
+	if (!cpuidle_data)
+		return -ENOMEM;
+
 	mutex_lock(&genpd->lock);
 
 	if (genpd->cpuidle_data) {
 		ret = -EEXIST;
-		goto out;
-	}
-	cpuidle_data = kzalloc(sizeof(*cpuidle_data), GFP_KERNEL);
-	if (!cpuidle_data) {
-		ret = -ENOMEM;
-		goto out;
+		goto err_drv;
 	}
+
 	cpuidle_drv = cpuidle_driver_ref();
 	if (!cpuidle_drv) {
 		ret = -ENODEV;

In preparation for supporting IRQ-safe domains, allocate domain data outside the domain locks. These functions are not called in an atomic context, so we can always allocate memory using GFP_KERNEL. By allocating memory before the locks, we can safely lock the domain using spinlocks instead of mutexes.

Cc Ulf Hansson <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Krzysztof Koz?owski <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/base/power/domain.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

drivers/base/power/domain.c +15 -16

index 7666a1c..5fd1306 100644
@@ -1382,13 +1382,17 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
                           struct generic_pm_domain *subdomain)
 {
-       struct gpd_link *link;
+       struct gpd_link *link, *itr;
        int ret = 0;
 
        if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(subdomain)
            || genpd == subdomain)
                return -EINVAL;
 
+       link = kzalloc(sizeof(*link), GFP_KERNEL);
+       if (!link)
+               return -ENOMEM;
+
        mutex_lock(&genpd->lock);
        mutex_lock_nested(&subdomain->lock, SINGLE_DEPTH_NESTING);
 

@@ -1398,18 +1402,13 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
                goto out;
        }
 
-       list_for_each_entry(link, &genpd->master_links, master_node) {
-               if (link->slave == subdomain && link->master == genpd) {
+       list_for_each_entry(itr, &genpd->master_links, master_node) {
+               if (itr->slave == subdomain && itr->master == genpd) {
                        ret = -EINVAL;
                        goto out;
                }
        }
 
-       link = kzalloc(sizeof(*link), GFP_KERNEL);
-       if (!link) {
-               ret = -ENOMEM;
-               goto out;
-       }
        link->master = genpd;
        list_add_tail(&link->master_node, &genpd->master_links);
        link->slave = subdomain;

@@ -1420,7 +1419,8 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
  out:
        mutex_unlock(&subdomain->lock);
        mutex_unlock(&genpd->lock);
-
+       if (ret)
+               kfree(link);
        return ret;
 }
 

@@ -1511,17 +1511,17 @@ int pm_genpd_attach_cpuidle(struct generic_pm_domain *genpd, int state)
        if (IS_ERR_OR_NULL(genpd) || state < 0)
                return -EINVAL;
 
+       cpuidle_data = kzalloc(sizeof(*cpuidle_data), GFP_KERNEL);
+       if (!cpuidle_data)
+               return -ENOMEM;
+
        mutex_lock(&genpd->lock);
 
        if (genpd->cpuidle_data) {
                ret = -EEXIST;
-               goto out;
-       }
-       cpuidle_data = kzalloc(sizeof(*cpuidle_data), GFP_KERNEL);
-       if (!cpuidle_data) {
-               ret = -ENOMEM;
-               goto out;
+               goto err_drv;
        }
+
        cpuidle_drv = cpuidle_driver_ref();
        if (!cpuidle_drv) {
                ret = -ENODEV;
--

2.1.4

Remove check for driver of a device, for runtime PM. Device may be suspended without an explicit driver. This check seems to be vestigial and incorrect in the current context.

Reviewed-by Krzysztof Kozlowski <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/base/power/domain.c +1 -2

index 5fd1306..ef8d19f 100644
@@ -394,8 +394,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 		if (stat > PM_QOS_FLAGS_NONE)
 			return -EBUSY;
 
-		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-		    || pdd->dev->power.irq_safe))
+		if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
 			not_suspended++;
 	}

Quoted 3 lines · Lina Iyer

Remove check for driver of a device, for runtime PM. Device may be suspended without an explicit driver. This check seems to be vestigial and incorrect in the current context.

This one should probably have been RFC.

For a little more context here, this was uncovered when experimenting with using runtime PM for CPU devices which don't have a dev->driver.

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Thoughts?

Hi Kevin,

Quoted 16 lines · Kevin Hilman

Quoted 3 lines · Lina Iyer

Remove check for driver of a device, for runtime PM. Device may be suspended without an explicit driver. This check seems to be vestigial and incorrect in the current context.

This one should probably have been RFC.

For a little more context here, this was uncovered when experimenting with using runtime PM for CPU devices which don't have a dev->driver.

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Quoted 1 line

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines · Kevin Hilman

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

That's correct, and in the proof-of-concept stuff I hacked up and in Lina's series, the CPU "devices" do indeed to this. Without that, they wouldn't end up ever taking this codepath through genpd's runtime_suspend and power_off hooks.

Also, I'm not sure if your comment was meant to be an objection to the patch? or if you're OK with it.

Hi Kevin,

Quoted 18 lines · Kevin Hilman

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines · Kevin Hilman

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

That's correct, and in the proof-of-concept stuff I hacked up and in Lina's series, the CPU "devices" do indeed to this. Without that, they wouldn't end up ever taking this codepath through genpd's runtime_suspend and power_off hooks.

Quoted 2 lines

Also, I'm not sure if your comment was meant to be an objection to the patch? or if you're OK with it.

My comment was purely meant as a response to "it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM".

I have no objections to the patch (read: I'm not sufficiently familiar with the code to make educated guesses about the side effects of this change ;-).

Quoted 22 lines · Geert Uytterhoeven

Quoted 20 lines · Kevin Hilman

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines · Kevin Hilman

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

That's correct, and in the proof-of-concept stuff I hacked up and in Lina's series, the CPU "devices" do indeed to this. Without that, they wouldn't end up ever taking this codepath through genpd's runtime_suspend and power_off hooks.

Also, I'm not sure if your comment was meant to be an objection to the patch? or if you're OK with it.

Quoted 2 lines

My comment was purely meant as a response to "it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM".

Right, so if the PM domain does the pm_runtime_enable() for these "simple" devices without drivers, they can still exist without a driver, and the PM domain doing all the magic.

Hi Kevin,

Quoted 27 lines · Kevin Hilman

Quoted 24 lines · Geert Uytterhoeven

Quoted 13 lines · Kevin Hilman

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines · Kevin Hilman

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

Quoted 7 lines

That's correct, and in the proof-of-concept stuff I hacked up and in Lina's series, the CPU "devices" do indeed to this. Without that, they wouldn't end up ever taking this codepath through genpd's runtime_suspend and power_off hooks.

Also, I'm not sure if your comment was meant to be an objection to the patch? or if you're OK with it.

My comment was purely meant as a response to "it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM".

Quoted 3 lines

Right, so if the PM domain does the pm_runtime_enable() for these "simple" devices without drivers, they can still exist without a driver, and the PM domain doing all the magic.

Is it possible to let the PM Domain do the pm_runtime_enable() itself in the absence of a driver?

If yes, I wouldn't have needed simple-pm-bus.c.
What if a driver is bound later?

Quoted 30 lines · Geert Uytterhoeven

Hi Kevin,

Quoted 27 lines · Kevin Hilman

Quoted 24 lines · Geert Uytterhoeven

Quoted 13 lines · Kevin Hilman

Quoted 11 lines · Geert Uytterhoeven

Quoted 6 lines · Kevin Hilman

This check might have made sense before PM domains, but with PM domains, it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM, so I think this check could/should be removed.

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

Quoted 7 lines

That's correct, and in the proof-of-concept stuff I hacked up and in Lina's series, the CPU "devices" do indeed to this. Without that, they wouldn't end up ever taking this codepath through genpd's runtime_suspend and power_off hooks.

Also, I'm not sure if your comment was meant to be an objection to the patch? or if you're OK with it.

My comment was purely meant as a response to "it's entirely possible to have a simple device without a driver and the PM domain handles all the necesary PM".

Quoted 6 lines

Quoted 3 lines

Right, so if the PM domain does the pm_runtime_enable() for these "simple" devices without drivers, they can still exist without a driver, and the PM domain doing all the magic.

Is it possible to let the PM Domain do the pm_runtime_enable() itself in the absence of a driver?

Well, I suppose it's possible, not sure it's recommended. :)

Quoted 2 lines

If yes, I wouldn't have needed simple-pm-bus.c.
What if a driver is bound later?

Yeah, you're approach is better.

Quoted 45 lines · Kevin Hilman

Quoted 35 lines · Geert Uytterhoeven

Quoted 34 lines · Kevin Hilman

Quoted 31 lines · Geert Uytterhoeven

Quoted 16 lines · Kevin Hilman

Quoted 9 lines · Geert Uytterhoeven

Quoted 7 lines · Kevin Hilman

This check might have made sense before PM domains, but with PM domains,
it's entirely possible to have a simple device without a driver and the
PM domain handles all the necesary PM, so I think this check could/should be removed.

Quoted 5 lines

Quoted 1 line

Thoughts?

Simple devices without a driver aren't handled automatically. At minimum, the driver should call pm_runtime_enable(), cfr. drivers/bus/simple-pm-bus.c.

Quoted 8 lines

That's correct, and in the proof-of-concept stuff I hacked up and in Lina's series, the CPU "devices" do indeed to this. Without that, they
wouldn't end up ever taking this codepath through genpd's runtime_suspend and power_off hooks.

Also, I'm not sure if your comment was meant to be an objection to the patch? or if you're OK with it.

My comment was purely meant as a response to "it's entirely possible to have a
simple device without a driver and the PM domain handles all the necesary PM".

Quoted 6 lines

Quoted 3 lines

Right, so if the PM domain does the pm_runtime_enable() for these "simple" devices without drivers, they can still exist without a driver, and the PM domain doing all the magic.

Is it possible to let the PM Domain do the pm_runtime_enable() itself in the absence of a driver?

Well, I suppose it's possible, not sure it's recommended. :)

Quoted 5 lines

Quoted 2 lines

If yes, I wouldn't have needed simple-pm-bus.c.
What if a driver is bound later?

Yeah, you're approach is better.

I am not sure I understand the approach? Initialize the CPU devices as "simple-pm-bus" compatible?

Remove check for driver of a device, for runtime PM. Device may be suspended without an explicit driver. This check seems to be vestigial and incorrect in the current context.

Reviewed-by Krzysztof Kozlowski <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/base/power/domain.c +1 -3

index 5fd1306..ef8d19f 100644
@@ -394,8 +394,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
                if (stat > PM_QOS_FLAGS_NONE)
                        return -EBUSY;
 
-               if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-                   || pdd->dev->power.irq_safe))
+               if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
                        not_suspended++;
        }
 
--

2.1.4

Generic Power Domains currently support turning on/off only in process context. This prevents the usage of PM domains for domains that could be powered on/off in a context where IRQs are disabled. Many such domains exist today and do not get powered off, when the IRQ safe devices in that domain are powered off, because of this limitation.

However, not all domains can operate in IRQ safe contexts. Genpd therefore, has to support both cases where the domain may or may not operate in IRQ safe contexts. Configuring genpd to use an appropriate lock for that domain, would allow domains that have IRQ safe devices to runtime suspend and resume, in atomic context.

To achieve domain specific locking, set the domain's ->flag to GENPD_FLAG_IRQ_SAFE while defining the domain. This indicates that genpd should use a spinlock instead of a mutex for locking the domain. Locking is abstracted through genpd_lock() and genpd_unlock() functions that use the flag to determine the appropriate lock to be used for that domain. Domains that have lower latency to suspend and resume and can operate with IRQs disabled may now be able to save power, when the component devices and sub-domains are idle at runtime.

The restriction this imposes on the domain hierarchy is that sub-domains and all devices in the IRQ safe domain's hierarchy also have to be IRQ safe, so that we dont try to lock a mutex, while holding a spinlock. Non-IRQ safe domains may continue to have devices and sub-domains that may or may not be IRQ safe.

Cc Ulf Hansson <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Krzysztof Koz?owski <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 Documentation/power/devices.txt |  11 ++-
 drivers/base/power/domain.c     | 201 +++++++++++++++++++++++++++++++---------
 include/linux/pm_domain.h       |  11 ++-
 3 files changed, 178 insertions(+), 45 deletions(-)

Documentation/power/devices.txt +10 -1

index 8ba6625..6d8318c 100644
@@ -607,7 +607,16 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain.
+
+Devices, by default, operate in process context and if a device can operate in
+IRQ safe context, has to be explicitly set as IRQ safe. Power domains by
+default, operate in process context but could have devices that are IRQ safe.
+Such power domains cannot be powered on/off during runtime PM. On the other
+hand, an IRQ safe PM domain that can be powered on/off and suspend or resumed
+in an atomic context, may contain IRQ safe devices. Such domains may only
+contain IRQ safe devices or IRQ safe sub-domains.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,

drivers/base/power/domain.c +0 -0

index ef8d19f..221feb0 100644

drivers/base/power/domain.c +70 -4

+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	if (subclass > 0)
+		spin_lock_irqsave_nested(&genpd->slock, flags, subclass);
+	else
+		spin_lock_irqsave(&genpd->slock, flags);
+
+	genpd->lock_flags = flags;
+	return 0;
+}
+
+static inline void genpd_unlock_noirq(struct generic_pm_domain *genpd)
+	__releases(&genpd->slock)
+{
+	spin_unlock_irqrestore(&genpd->slock, genpd->lock_flags);
+}
+
+static inline int genpd_lock_irq(struct generic_pm_domain *genpd,
+					unsigned int subclass)
+	__acquires(&genpd->mlock)
+{
+	if (subclass > 0)
+		mutex_lock_nested(&genpd->mlock, subclass);
+	else
+		mutex_lock(&genpd->mlock);
+	return 0;
+}
+
+static inline int genpd_lock_interruptible_irq(struct generic_pm_domain *genpd)
+	__acquires(&genpd->mlock)
+{
+	return mutex_lock_interruptible(&genpd->mlock);
+}
+
+static inline void genpd_unlock_irq(struct generic_pm_domain *genpd)
+	__releases(&genpd->mlock)
+{
+	mutex_unlock(&genpd->mlock);
+}
+
+static inline int genpd_lock(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_lock_noirq(genpd, 0)
+			: genpd_lock_irq(genpd, 0);
+}
+
+static inline int genpd_lock_nested(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_lock_noirq(genpd, SINGLE_DEPTH_NESTING)
+			: genpd_lock_irq(genpd, SINGLE_DEPTH_NESTING);
+}
+
+static inline int genpd_lock_interruptible(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_lock_noirq(genpd, 0)
+			: genpd_lock_interruptible_irq(genpd);
+}
+
+static inline void genpd_unlock(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_unlock_noirq(genpd)
+			: genpd_unlock_irq(genpd);
+}
+
 static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;

@@ -273,9 +341,9 @@ int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
 	int ret;
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	ret = __pm_genpd_poweron(genpd);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 	return ret;
 }
 

@@ -335,9 +403,9 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
 		spin_unlock_irq(&dev->power.lock);
 
 		if (!IS_ERR(genpd)) {
-			mutex_lock(&genpd->lock);
+			genpd_lock(genpd);
 			genpd->max_off_time_changed = true;
-			mutex_unlock(&genpd->lock);
+			genpd_unlock(genpd);
 		}
 
 		dev = dev->parent;

@@ -394,7 +462,13 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 		if (stat > PM_QOS_FLAGS_NONE)
 			return -EBUSY;

drivers/base/power/domain.c +2 -2

 	}
 

@@ -460,9 +534,9 @@ static void genpd_power_off_work_fn(struct work_struct *work)
 
 	genpd = container_of(work, struct generic_pm_domain, power_off_work);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	pm_genpd_poweroff(genpd);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 }
 
 /**

@@ -500,17 +574,19 @@ static int pm_genpd_runtime_suspend(struct device *dev)
 	}
 

drivers/base/power/domain.c +16 -12

-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
+
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	return 0;
 }

@@ -535,15 +611,18 @@ static int pm_genpd_runtime_resume(struct device *dev)
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	/* If power.irq_safe, the PM domain is never powered off. */
-	if (dev->power.irq_safe) {
+	/*
+	 * As we dont power off a non IRQ safe domain, which holds
+	 * an IRQ safe device, we dont need to restore power to it.
+	 */
+	if (dev->power.irq_safe && !genpd->irq_safe) {
 		timed = false;
 		goto out;
 	}
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	ret = __pm_genpd_poweron(genpd);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (ret)
 		return ret;

@@ -743,14 +822,14 @@ static int pm_genpd_prepare(struct device *dev)
 	if (resume_needed(dev, genpd))
 		pm_runtime_resume(dev);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count++ == 0) {
 		genpd->suspended_count = 0;
 		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
 	}
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (genpd->suspend_power_off) {
 		pm_runtime_put_noidle(dev);

@@ -768,12 +847,12 @@ static int pm_genpd_prepare(struct device *dev)
 
 	ret = pm_generic_prepare(dev);
 	if (ret) {
-		mutex_lock(&genpd->lock);
+		genpd_lock(genpd);
 
 		if (--genpd->prepared_count == 0)
 			genpd->suspend_power_off = false;
 
-		mutex_unlock(&genpd->lock);
+		genpd_unlock(genpd);
 		pm_runtime_enable(dev);
 	}
 

@@ -1130,13 +1209,13 @@ static void pm_genpd_complete(struct device *dev)
 	if (IS_ERR(genpd))
 		return;
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	run_complete = !genpd->suspend_power_off;
 	if (--genpd->prepared_count == 0)
 		genpd->suspend_power_off = false;
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (run_complete) {
 		pm_generic_complete(dev);

drivers/base/power/domain.c +8 -5

+		return -EINVAL;
+	}
+
 	gpd_data = genpd_alloc_dev_data(dev, genpd, td);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;

@@ -1301,7 +1386,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (ret)
 		genpd_free_dev_data(dev, gpd_data);

@@ -1345,7 +1430,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;

@@ -1360,14 +1445,14 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 
 	list_del_init(&pdd->list_node);
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	genpd_free_dev_data(dev, gpd_data);
 
 	return 0;
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 	dev_pm_qos_add_notifier(dev, &gpd_data->nb);
 
 	return ret;

@@ -1388,12 +1473,24 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 	    || genpd == subdomain)

Quoted 27 lines · Lina Iyer

Generic Power Domains currently support turning on/off only in process context. This prevents the usage of PM domains for domains that could be powered on/off in a context where IRQs are disabled. Many such domains exist today and do not get powered off, when the IRQ safe devices in that domain are powered off, because of this limitation.

However, not all domains can operate in IRQ safe contexts. Genpd therefore, has to support both cases where the domain may or may not operate in IRQ safe contexts. Configuring genpd to use an appropriate lock for that domain, would allow domains that have IRQ safe devices to runtime suspend and resume, in atomic context.

To achieve domain specific locking, set the domain's ->flag to GENPD_FLAG_IRQ_SAFE while defining the domain. This indicates that genpd should use a spinlock instead of a mutex for locking the domain. Locking is abstracted through genpd_lock() and genpd_unlock() functions that use the flag to determine the appropriate lock to be used for that domain. Domains that have lower latency to suspend and resume and can operate with IRQs disabled may now be able to save power, when the component devices and sub-domains are idle at runtime.

The restriction this imposes on the domain hierarchy is that sub-domains and all devices in the IRQ safe domain's hierarchy also have to be IRQ safe, so that we dont try to lock a mutex, while holding a spinlock. Non-IRQ safe domains may continue to have devices and sub-domains that may or may not be IRQ safe.

Quoted 6 lines

Cc Ulf Hansson <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Rafael J. Wysocki <[email protected]>

Cc Geert Uytterhoeven <[email protected]>

Cc Krzysztof Kozłowski <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

Approach seems good to me, some cosmetic issues below...

Quoted 36 lines

---
 Documentation/power/devices.txt |  11 ++-
 drivers/base/power/domain.c     | 201 +++++++++++++++++++++++++++++++---------
 include/linux/pm_domain.h       |  11 ++-
 3 files changed, 178 insertions(+), 45 deletions(-)

Documentation/power/devices.txt +10 -1

index 8ba6625..6d8318c 100644
@@ -607,7 +607,16 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state
 together, by turning the shared power resource on.  A set of devices with this
-property is often referred to as a power domain.
+property is often referred to as a power domain. A power domain may also be
+nested inside another power domain.
+
+Devices, by default, operate in process context and if a device can operate in
+IRQ safe context, has to be explicitly set as IRQ safe. Power domains by
+default, operate in process context but could have devices that are IRQ safe.
+Such power domains cannot be powered on/off during runtime PM. On the other
+hand, an IRQ safe PM domain that can be powered on/off and suspend or resumed
+in an atomic context, may contain IRQ safe devices. Such domains may only
+contain IRQ safe devices or IRQ safe sub-domains.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,

drivers/base/power/domain.c +0 -0

index ef8d19f..221feb0 100644
@@ -53,6 +53,74 @@
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 

drivers/base/power/domain.c line 57

Quoted 2 lines

drivers/base/power/domain.c +2 -0

+static inline int genpd_lock_noirq(struct generic_pm_domain *genpd,
+					unsigned int subclass)

I don't like the _noirq naming, as in the context of suspend/resume, that means no *device* IRQs, not no IRQs at all. Maybe _atomic is better, or _nosleep? or ...

Quoted 97 lines

drivers/base/power/domain.c +70 -4

+	__acquires(&genpd->slock)
+{
+	unsigned long flags;
+
+	if (subclass > 0)
+		spin_lock_irqsave_nested(&genpd->slock, flags, subclass);
+	else
+		spin_lock_irqsave(&genpd->slock, flags);
+
+	genpd->lock_flags = flags;
+	return 0;
+}
+
+static inline void genpd_unlock_noirq(struct generic_pm_domain *genpd)
+	__releases(&genpd->slock)
+{
+	spin_unlock_irqrestore(&genpd->slock, genpd->lock_flags);
+}
+
+static inline int genpd_lock_irq(struct generic_pm_domain *genpd,
+					unsigned int subclass)
+	__acquires(&genpd->mlock)
+{
+	if (subclass > 0)
+		mutex_lock_nested(&genpd->mlock, subclass);
+	else
+		mutex_lock(&genpd->mlock);
+	return 0;
+}
+
+static inline int genpd_lock_interruptible_irq(struct generic_pm_domain *genpd)
+	__acquires(&genpd->mlock)
+{
+	return mutex_lock_interruptible(&genpd->mlock);
+}
+
+static inline void genpd_unlock_irq(struct generic_pm_domain *genpd)
+	__releases(&genpd->mlock)
+{
+	mutex_unlock(&genpd->mlock);
+}
+
+static inline int genpd_lock(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_lock_noirq(genpd, 0)
+			: genpd_lock_irq(genpd, 0);
+}
+
+static inline int genpd_lock_nested(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_lock_noirq(genpd, SINGLE_DEPTH_NESTING)
+			: genpd_lock_irq(genpd, SINGLE_DEPTH_NESTING);
+}
+
+static inline int genpd_lock_interruptible(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_lock_noirq(genpd, 0)
+			: genpd_lock_interruptible_irq(genpd);
+}
+
+static inline void genpd_unlock(struct generic_pm_domain *genpd)
+{
+	return genpd->irq_safe ? genpd_unlock_noirq(genpd)
+			: genpd_unlock_irq(genpd);
+}
+
 static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;

@@ -273,9 +341,9 @@ int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
 	int ret;
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	ret = __pm_genpd_poweron(genpd);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 	return ret;
 }
 

@@ -335,9 +403,9 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb,
 		spin_unlock_irq(&dev->power.lock);
 
 		if (!IS_ERR(genpd)) {
-			mutex_lock(&genpd->lock);
+			genpd_lock(genpd);
 			genpd->max_off_time_changed = true;
-			mutex_unlock(&genpd->lock);
+			genpd_unlock(genpd);
 		}
 
 		dev = dev->parent;

@@ -394,7 +462,13 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
 		if (stat > PM_QOS_FLAGS_NONE)
 			return -EBUSY;
 

drivers/base/power/domain.c static int pm_genpd_poweroff(struct generic_pm_domain *genpd) · line 472

Quoted 9 lines

drivers/base/power/domain.c +7 -1

-		if (!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)
+		/*
+		 * We do not want to power off the domain if the device is
+		 * not suspended or an IRQ safe device is part of this
+		 * non-IRQ safe domain.
+		 */
+		if (!pm_runtime_suspended(pdd->dev) ||
+			(pdd->dev->power.irq_safe && !genpd->irq_safe))
 			not_suspended++;

The !irq_safe check should maybe spit a warning via WARN_ONCE(), as this would be useful in debugging domains that are not shutting down.

Quoted 1 line

drivers/base/power/domain.c +0 -0

 	}

Quoted 16 lines

drivers/base/power/domain.c +2 -2

@@ -460,9 +534,9 @@ static void genpd_power_off_work_fn(struct work_struct *work)
 
 	genpd = container_of(work, struct generic_pm_domain, power_off_work);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	pm_genpd_poweroff(genpd);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 }
 
 /**

@@ -500,17 +574,19 @@ static int pm_genpd_runtime_suspend(struct device *dev)
 	}
 

drivers/base/power/domain.c static int pm_genpd_runtime_suspend(struct device *dev) · line 582

Quoted 10 lines

drivers/base/power/domain.c +4 -3

 	/*
-	 * If power.irq_safe is set, this routine will be run with interrupts
-	 * off, so it can't use mutexes.
+	 * If power.irq_safe is set, this routine may be run with
+	 * IRQ disabled, so suspend only if the power domain is
+	 * irq_safe.
 	 */
-	if (dev->power.irq_safe)
+	if (dev->power.irq_safe && !genpd->irq_safe)
 		return 0;

Similarily, a WARN_ONCE() will probably be helpful here.

Quoted 86 lines

drivers/base/power/domain.c +16 -12

-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
+
 	genpd->in_progress++;
 	pm_genpd_poweroff(genpd);
 	genpd->in_progress--;
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	return 0;
 }

@@ -535,15 +611,18 @@ static int pm_genpd_runtime_resume(struct device *dev)
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	/* If power.irq_safe, the PM domain is never powered off. */
-	if (dev->power.irq_safe) {
+	/*
+	 * As we dont power off a non IRQ safe domain, which holds
+	 * an IRQ safe device, we dont need to restore power to it.
+	 */
+	if (dev->power.irq_safe && !genpd->irq_safe) {
 		timed = false;
 		goto out;
 	}
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 	ret = __pm_genpd_poweron(genpd);
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (ret)
 		return ret;

@@ -743,14 +822,14 @@ static int pm_genpd_prepare(struct device *dev)
 	if (resume_needed(dev, genpd))
 		pm_runtime_resume(dev);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count++ == 0) {
 		genpd->suspended_count = 0;
 		genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF;
 	}
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (genpd->suspend_power_off) {
 		pm_runtime_put_noidle(dev);

@@ -768,12 +847,12 @@ static int pm_genpd_prepare(struct device *dev)
 
 	ret = pm_generic_prepare(dev);
 	if (ret) {
-		mutex_lock(&genpd->lock);
+		genpd_lock(genpd);
 
 		if (--genpd->prepared_count == 0)
 			genpd->suspend_power_off = false;
 
-		mutex_unlock(&genpd->lock);
+		genpd_unlock(genpd);
 		pm_runtime_enable(dev);
 	}
 

@@ -1130,13 +1209,13 @@ static void pm_genpd_complete(struct device *dev)
 	if (IS_ERR(genpd))
 		return;
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	run_complete = !genpd->suspend_power_off;
 	if (--genpd->prepared_count == 0)
 		genpd->suspend_power_off = false;
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (run_complete) {
 		pm_generic_complete(dev);

@@ -1280,11 +1359,17 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
 		return -EINVAL;
 

drivers/base/power/domain.c int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, · line 1364

Quoted 3 lines

drivers/base/power/domain.c +3 -0

+	if (genpd->irq_safe && !dev->power.irq_safe) {
+		dev_err(dev,
+			"Devices in an IRQ safe domain have to be IRQ safe.\n");

This should also dump the PM domain, something like:

dev_err(dev, "PM domain %s is IRQ safe; device has to be IRQ safe.\n")

Quoted 51 lines

drivers/base/power/domain.c +8 -5

+		return -EINVAL;
+	}
+
 	gpd_data = genpd_alloc_dev_data(dev, genpd, td);
 	if (IS_ERR(gpd_data))
 		return PTR_ERR(gpd_data);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;

@@ -1301,7 +1386,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	if (ret)
 		genpd_free_dev_data(dev, gpd_data);

@@ -1345,7 +1430,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 	gpd_data = to_gpd_data(pdd);
 	dev_pm_qos_remove_notifier(dev, &gpd_data->nb);
 
-	mutex_lock(&genpd->lock);
+	genpd_lock(genpd);
 
 	if (genpd->prepared_count > 0) {
 		ret = -EAGAIN;

@@ -1360,14 +1445,14 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 
 	list_del_init(&pdd->list_node);
 
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 
 	genpd_free_dev_data(dev, gpd_data);
 
 	return 0;
 
  out:
-	mutex_unlock(&genpd->lock);
+	genpd_unlock(genpd);
 	dev_pm_qos_add_notifier(dev, &gpd_data->nb);
 
 	return ret;

@@ -1388,12 +1473,24 @@ int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 	    || genpd == subdomain)
 		return -EINVAL;
 

drivers/base/power/domain.c int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, · line 1483

Quoted 8 lines

drivers/base/power/domain.c +8 -0

+	/*
+	 * If the domain can be powered on/off in an IRQ safe
+	 * context, ensure that the subdomain can also be
+	 * powered on/off in that context.
+	 */
+	if (genpd->irq_safe && !subdomain->irq_safe) {
+		WARN("Sub-domain (%s) in an IRQ-safe domain (%s)"
+				"have to be IRQ safe\n",

s/have/has/

drivers/base/power/domain.c int pm_genpd_add_subdomain(struct generic_pm_domain *genpd, · line 1487

Quoted 4 lines

drivers/base/power/domain.c +4 -0

+				subdomain->name, genpd->name);
+		return -EINVAL;
+	}
+

Kevin

Documentation/power/devices.txt +0 -0

index 8ba6625..6d8318c 100644
@@ -607,7 +607,16 @@ individually.  Instead, a set of devices sharing a power resource can be put
 into a low-power state together at the same time by turning off the shared
 power resource.  Of course, they also need to be put into the full-power state

Documentation/power/devices.txt +2 -0

+in an atomic context, may contain IRQ safe devices. Such domains may only
+contain IRQ safe devices or IRQ safe sub-domains.
 
 Support for power domains is provided through the pm_domain field of struct
 device.  This field is a pointer to an object of type struct dev_pm_domain,

drivers/base/power/domain.c +3 -0

index ef8d19f..221feb0 100644
@@ -53,6 +53,74 @@
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
+static inline int genpd_lock_noirq(struct generic_pm_domain *genpd,
+					unsigned int subclass)
+	__acquires(&genpd->slock)

drivers/base/power/domain.c +7 -0

+}
+
+static inline void genpd_unlock_noirq(struct generic_pm_domain *genpd)
+	__releases(&genpd->slock)
+{
+	spin_unlock_irqrestore(&genpd->slock, genpd->lock_flags);
+}

drivers/base/power/domain.c +15 -0

+}
+
+static inline int genpd_lock_interruptible_irq(struct generic_pm_domain *genpd)
+	__acquires(&genpd->mlock)
+{
+	return mutex_lock_interruptible(&genpd->mlock);
+}
+
+static inline void genpd_unlock_irq(struct generic_pm_domain *genpd)
+	__releases(&genpd->mlock)
+{
+	mutex_unlock(&genpd->mlock);
+}
+
+static inline int genpd_lock(struct generic_pm_domain *genpd)

drivers/base/power/domain.c +1 -0

+
 static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;

cpu_cluster_pm_exit() must be sent after cpu_cluster_pm_enter() has been sent for the cluster and before any cpu_pm_exit() notifications are sent for any CPU.

Cc Nicolas Pitre <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
 kernel/cpu_pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/cpu_pm.c +1 -1

index 9656a3c..009cc9a 100644
@@ -180,7 +180,7 @@ EXPORT_SYMBOL_GPL(cpu_cluster_pm_enter);
  * low power state that may have caused some blocks in the same power domain
  * to reset.
  *
- * Must be called after cpu_pm_exit has been called on all cpus in the power
+ * Must be called after cpu_cluster_pm_enter has been called for the power
  * domain, and before cpu_pm_exit has been called on any cpu in the power
  * domain. Notified drivers can include VFP co-processor, interrupt controller
  * and its PM extensions, local CPU timers context save/restore which

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Quoted 2 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new

Quoted 15 lines

SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node.

Quoted 28 lines · Rob Herring

Quoted 11 lines · Lina Iyer

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 3 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node.

That's correct, the CPU nodes (and other nodes in the cluster like GIC, Coresight, etc.) would have phandles to the cluster power domain node.

But this series is meant to create the driver & binding for those cluster power domain(s), so the question is how exactly describe it.

Quoted 29 lines · Kevin Hilman

Quoted 28 lines · Rob Herring

Quoted 11 lines · Lina Iyer

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 6 lines

Quoted 3 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node.

That's correct, the CPU nodes (and other nodes in the cluster like GIC, Coresight, etc.) would have phandles to the cluster power domain node.

Indeed.

Quoted 2 lines

But this series is meant to create the driver & binding for those cluster power domain(s), so the question is how exactly describe it.

I don't think I can add an "arm,pd" compatible property to e.g. a2sl (for CA15-CPUx) and a3sm (for CA15-SCU) in arch/arm/boot/dts/r8a73a4.dtsi, as these are just subdomains in a power domain hierarchy, all driven by a single hardware block.

I can call e.g. a special registration method, or set up some ops pointer, for the a2sl and a3sm subdomains from within the "renesas,sysc-rmobile" driver.

Quoted 58 lines · Geert Uytterhoeven

Quoted 41 lines · Kevin Hilman

Quoted 39 lines · Rob Herring

Quoted 29 lines · Lina Iyer

Documentation/devicetree/bindings/arm/cpudomains.txt new file +3 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In

ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also

Documentation/devicetree/bindings/arm/cpudomains.txt new file +3 -0

+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM

domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

(quoted diff) +2 -0

+
+The ARM CPU PM domain follows the same binding convention as any

generic PM

Documentation/devicetree/bindings/arm/cpudomains.txt new file +8 -0

+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM

Quoted 1 line

domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 7 lines

Quoted 3 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node.

That's correct, the CPU nodes (and other nodes in the cluster like GIC, Coresight, etc.) would have phandles to the cluster power domain node.

Indeed.

Quoted 2 lines

But this series is meant to create the driver & binding for those cluster power domain(s), so the question is how exactly describe it.

Quoted 9 lines

I don't think I can add an "arm,pd" compatible property to e.g. a2sl (for CA15-CPUx) and a3sm (for CA15-SCU) in arch/arm/boot/dts/r8a73a4.dtsi, as these are just subdomains in a power domain hierarchy, all driven by a single hardware block.

I can call e.g. a special registration method, or set up some ops pointer, for the a2sl and a3sm subdomains from within the "renesas,sysc-rmobile" driver.

I was hoping the macro would help in such a case. But since your domain cannot be defined as arm,pd (could you explain why, I seem to missing the obvious) would it help if I export a function like that the renesas,sysc-rmobile driver could call and setup the CPU PM domain? There is catch to it though.

The problem is -

To be generic and not have every driver write code to do this generic functionality, the common code would want to instantiate the arm_pm_domain and therefore the embedded genpd. A pointer instead of actual object, would mean maintaining a list and iterating through it, everytime the domain is suspended/resumed. With an object, I could just do container_of and get the arm_pd. But, we also want to give the platform an option to define certain aspects of the CPU's generic PM domain like the flags, genpd callbacks etc, before the genpd is registered with the framework.

Would such a function work for you? What does everyone think about the @template?

struct generic_pm_domain *of_arm_cpu_domain(struct device_node *dn,

struct of_arm_pd_ops *ops, struct generic_pm_domain
*template)

{
       struct arm_pm_domain *pd;

       if (!of_device_is_available(dn))
               return NULL;

        /* This creates the memory for pd and setup basic stuff */
       pd = setup_arm_pd(dn);

        /* copy the platform's template genpd over to the pd's genpd */
       memcpy(&pd.genpd, template, sizeof(*template));

        /*       * Now set up the additional ops and flags and register with
         * genpd framework
         */
       register_arm_pd(dn, pd);

        /*       * Returning the genpd back to the platform so it can be
added
         * as subdomains to other domains etc.
         */
       return &pd.genpd;
}

EXPORT_SYMBOL(of_arm_cpu_domain);

The catch is that platform drivers have to provide a template for the genpd. The @template would never be registered, but would just be used to create the common code's genpd.

Any other ideas?

Thanks,
Lina

Quoted 73 lines · Lina Iyer

Quoted 55 lines · Geert Uytterhoeven

Quoted 39 lines · Kevin Hilman

Quoted 38 lines · Rob Herring

Quoted 29 lines · Lina Iyer

Documentation/devicetree/bindings/arm/cpudomains.txt new file +3 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In

ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also

Documentation/devicetree/bindings/arm/cpudomains.txt new file +3 -0

+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM

domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

(quoted diff) +2 -0

+
+The ARM CPU PM domain follows the same binding convention as any

generic PM

Documentation/devicetree/bindings/arm/cpudomains.txt new file +8 -0

+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM

Quoted 1 line

domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 7 lines

Quoted 3 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node.

That's correct, the CPU nodes (and other nodes in the cluster like GIC, Coresight, etc.) would have phandles to the cluster power domain node.

Indeed.

Quoted 2 lines

But this series is meant to create the driver & binding for those cluster power domain(s), so the question is how exactly describe it.

Quoted 9 lines

I don't think I can add an "arm,pd" compatible property to e.g. a2sl (for CA15-CPUx) and a3sm (for CA15-SCU) in arch/arm/boot/dts/r8a73a4.dtsi, as these are just subdomains in a power domain hierarchy, all driven by a single hardware block.

I can call e.g. a special registration method, or set up some ops pointer, for the a2sl and a3sm subdomains from within the "renesas,sysc-rmobile" driver.

I was hoping the macro would help in such a case. But since your domain cannot be defined as arm,pd (could you explain why, I seem to missing the obvious) would it help if I export a function like that the renesas,sysc-rmobile driver could call and setup the CPU PM domain? There is catch to it though.

The problem is -

Quoted 9 lines

To be generic and not have every driver write code to do this generic functionality, the common code would want to instantiate the arm_pm_domain and therefore the embedded genpd. A pointer instead of actual object, would mean maintaining a list and iterating through it, everytime the domain is suspended/resumed. With an object, I could just do container_of and get the arm_pd. But, we also want to give the platform an option to define certain aspects of the CPU's generic PM domain like the flags, genpd callbacks etc, before the genpd is registered with the framework.

The problem here is what part of the hardware is generic? It is generic, but yet ARM specific (presumably not as Kevin pointed out)?

I'm not exactly clear what the problem is, but it seems you are after a common API/subsystem for managing power domains of CPU/clusters.

I fail to see how the problem is different than any other subsystem where you have a core providing common API with hardware specific drivers registering their ops with the core.

Quoted 5 lines

Would such a function work for you? What does everyone think about the @template?

struct generic_pm_domain *of_arm_cpu_domain(struct device_node *dn,

This is missing a verb. What does it do?

I don't think I really get what you are trying to solve to comment whether this looks good or not.

Rob

Quoted 35 lines

struct of_arm_pd_ops *ops, struct generic_pm_domain
*template)

{
       struct arm_pm_domain *pd;

       if (!of_device_is_available(dn))
               return NULL;

        /* This creates the memory for pd and setup basic stuff */
       pd = setup_arm_pd(dn);

        /* copy the platform's template genpd over to the pd's genpd */
       memcpy(&pd.genpd, template, sizeof(*template));

        /*       * Now set up the additional ops and flags and register with
         * genpd framework
         */
       register_arm_pd(dn, pd);

        /*       * Returning the genpd back to the platform so it can be
added
         * as subdomains to other domains etc.
         */
       return &pd.genpd;
}

EXPORT_SYMBOL(of_arm_cpu_domain);

The catch is that platform drivers have to provide a template for the genpd. The @template would never be registered, but would just be used to create the common code's genpd.

Any other ideas?

Thanks,
Lina

Define and add Generic PM domains (genpd) for CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

Cc Stephen Boyd <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Daniel Lezcano <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
Changes since v1:

- Function name changes and split out common code
- Use cpu,pd for now. Removed references to ARM. Open to recommendations.
- Still located in arch/arm/common/. May move to a more appropriate location.
- Platform drivers can directly call of_init_cpu_domain() without using compatibles.
- Now maintains a list of CPU PM domains.

arch/arm/common/domains.c new file +10 -0

+static int __init of_pm_domain_attach_cpus(void)
+{
+       int cpuid, ret;
+
+       /* Find any CPU nodes with a phandle to this power domain */
+       for_each_possible_cpu(cpuid) {
+               struct device *cpu_dev;
+               struct of_phandle_args pd_args;
+
+               cpu_dev = get_cpu_device(cpuid);

Quoted 35 lines · Lina Iyer

Define and add Generic PM domains (genpd) for CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

Cc Stephen Boyd <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Daniel Lezcano <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
Changes since v1:

- Function name changes and split out common code
- Use cpu,pd for now. Removed references to ARM. Open to recommendations.
- Still located in arch/arm/common/. May move to a more appropriate location.
- Platform drivers can directly call of_init_cpu_domain() without using compatibles.
- Now maintains a list of CPU PM domains.

[...]

Quoted 14 lines

arch/arm/common/domains.c new file +14 -0

+static int __init of_pm_domain_attach_cpus(void)
+{
+       int cpuid, ret;
+
+       /* Find any CPU nodes with a phandle to this power domain */
+       for_each_possible_cpu(cpuid) {
+               struct device *cpu_dev;
+               struct of_phandle_args pd_args;
+
+               cpu_dev = get_cpu_device(cpuid);
+               if (!cpu_dev) {
+                       pr_warn("%s: Unable to get device for CPU%d\n",
+                                       __func__, cpuid);
+                       return -ENODEV;

arch/arm/common/domains.c line 75

Quoted 6 lines

arch/arm/common/domains.c new file +6 -0

+               }
+
+               /*
+                * We are only interested in CPUs that can be attached to
+                * PM domains that are cpu,pd compatible.
+                */

Under what conditions would the power domain for a cpu not be cpu,pd compatible?

Why can't the driver handling the power domain register with gen_pd and the cpu_pd as the driver is going to be aware of which domains are for cpus.

While there could be h/w such that all power domains within a chip have a nice uniform programming model, I'd guess that is the exception, not the rule. First, chips I have worked on were not that way. CPU related and peripheral related domains are handled quite differently.

Second, often the actions on the CPU power domains don't take effect until a WFI, so you end up with a different programming sequence.

Quoted 59 lines · Rob Herring

Quoted 37 lines · Lina Iyer

Define and add Generic PM domains (genpd) for CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

Cc Stephen Boyd <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Daniel Lezcano <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
Changes since v1:

- Function name changes and split out common code
- Use cpu,pd for now. Removed references to ARM. Open to recommendations.
- Still located in arch/arm/common/. May move to a more appropriate location.
- Platform drivers can directly call of_init_cpu_domain() without using compatibles.
- Now maintains a list of CPU PM domains.

[...]

Quoted 16 lines

arch/arm/common/domains.c new file +16 -0

+static int __init of_pm_domain_attach_cpus(void)
+{
+       int cpuid, ret;
+
+       /* Find any CPU nodes with a phandle to this power domain */
+       for_each_possible_cpu(cpuid) {
+               struct device *cpu_dev;
+               struct of_phandle_args pd_args;
+
+               cpu_dev = get_cpu_device(cpuid);
+               if (!cpu_dev) {
+                       pr_warn("%s: Unable to get device for CPU%d\n",
+                                       __func__, cpuid);
+                       return -ENODEV;
+               }
+

Quoted 9 lines

arch/arm/common/domains.c line 75

Quoted 4 lines

arch/arm/common/domains.c new file +4 -0

+               /*
+                * We are only interested in CPUs that can be attached to
+                * PM domains that are cpu,pd compatible.
+                */

Under what conditions would the power domain for a cpu not be cpu,pd compatible?

Mostly never. But I dont want to assume and attach a CPU to its domain that I am not concerned with.

Quoted 3 lines

Why can't the driver handling the power domain register with gen_pd and the cpu_pd as the driver is going to be aware of which domains are for cpus.

They could and like Renesas they would. They could have an intricate hierarchy of domains that they may want to deal with in their platform drivers. Platforms could define the CPU devices as IRQ-safe and attach it to their domains. Ensure the reference count of the online and running CPUs are correct and they are good to go. They also would attach the CPU devices to the domain and everything would work as they would here. Its just repeated code across platforms that we are trying to avoid.

Quoted 5 lines

While there could be h/w such that all power domains within a chip have a nice uniform programming model, I'd guess that is the exception, not the rule. First, chips I have worked on were not that way. CPU related and peripheral related domains are handled quite differently.

Agreed. These are very SoC specific components and would require platform specific programming. But most SoCs, would need to do many other things like suspending debuggers, reducing clocks, GIC save and restore, cluster state determination etc that are only getting more generalized. We have generalized ARM CPU idle, I see this as the platform for the next set of power saving in an SoC.

Quoted 4 lines

Second, often the actions on the CPU power domains don't take effect until a WFI, so you end up with a different programming sequence.

How so? The last core going down (in this case, when the domain is suspended the last core is the last device that does a _put() on the domain) would determine and perform the power domain's programming sequence, in the context of the last CPU. A sequence that would only be effected in hardware when the the CPU executes WFI. I dont see why it would be any different than how it is today.

Quoted 69 lines · Lina Iyer

Quoted 68 lines · Rob Herring

Quoted 37 lines · Lina Iyer

Define and add Generic PM domains (genpd) for CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

Cc Stephen Boyd <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Daniel Lezcano <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Lorenzo Pieralisi <[email protected]>

Signed-off-by Kevin Hilman <[email protected]>

Signed-off-by Lina Iyer <[email protected]>

---
Changes since v1:

- Function name changes and split out common code
- Use cpu,pd for now. Removed references to ARM. Open to recommendations.
- Still located in arch/arm/common/. May move to a more appropriate location.
- Platform drivers can directly call of_init_cpu_domain() without using compatibles.
- Now maintains a list of CPU PM domains.

[...]

Quoted 10 lines

arch/arm/common/domains.c new file +10 -0

+static int __init of_pm_domain_attach_cpus(void)
+{
+       int cpuid, ret;
+
+       /* Find any CPU nodes with a phandle to this power domain */
+       for_each_possible_cpu(cpuid) {
+               struct device *cpu_dev;
+               struct of_phandle_args pd_args;
+
+               cpu_dev = get_cpu_device(cpuid);

arch/arm/common/domains.c line 75

Quoted 10 lines

arch/arm/common/domains.c new file +10 -0

+               if (!cpu_dev) {
+                       pr_warn("%s: Unable to get device for CPU%d\n",
+                                       __func__, cpuid);
+                       return -ENODEV;
+               }
+
+               /*
+                * We are only interested in CPUs that can be attached to
+                * PM domains that are cpu,pd compatible.
+                */

Under what conditions would the power domain for a cpu not be cpu,pd compatible?

Quoted 2 lines

Mostly never. But I dont want to assume and attach a CPU to its domain that I am not concerned with.

Which is why the power controller driver should tell you.

Quoted 3 lines

Quoted 3 lines

Why can't the driver handling the power domain register with gen_pd and the cpu_pd as the driver is going to be aware of which domains are for cpus.

Quoted 9 lines

They could and like Renesas they would. They could have an intricate hierarchy of domains that they may want to deal with in their platform drivers. Platforms could define the CPU devices as IRQ-safe and attach it to their domains. Ensure the reference count of the online and running CPUs are correct and they are good to go. They also would attach the CPU devices to the domain and everything would work as they would here. Its just repeated code across platforms that we are trying to avoid.

I agree that we want to have core code doing all that setup, But that has nothing to do with needing to have a DT property. The driver just needs to tell you the list of cpu power domains and the associated cpus they want the core to manage. Then it is up to you to do the rest of the setup.

So I really don't think we need a DT binding here.

Quoted 11 lines

Quoted 11 lines

While there could be h/w such that all power domains within a chip have a nice uniform programming model, I'd guess that is the exception, not the rule. First, chips I have worked on were not that way. CPU related and peripheral related domains are handled quite differently.

Second, often the actions on the CPU power domains don't take effect until a WFI, so you end up with a different programming sequence.

Quoted 6 lines

How so? The last core going down (in this case, when the domain is suspended the last core is the last device that does a _put() on the domain) would determine and perform the power domain's programming sequence, in the context of the last CPU. A sequence that would only be effected in hardware when the the CPU executes WFI. I dont see why it would be any different than how it is today.

I just mean that for a peripheral (e.g a SATA controller), you simply quiesce the device and driver and shut off power. With a cpu or cpu related component, you can't really shut it off until you stop running. So cpu domains are special.

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

Quoted 47 lines · Rob Herring

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Quoted 48 lines · Lorenzo Pieralisi

Quoted 47 lines · Rob Herring

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

Quoted 65 lines · Lina Iyer

Quoted 48 lines · Lorenzo Pieralisi

Quoted 47 lines · Rob Herring

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Quoted 6 lines

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

What activities ? CPU PM notifiers ?

Quoted 65 lines · Lina Iyer

Quoted 48 lines · Lorenzo Pieralisi

Quoted 47 lines · Rob Herring

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Quoted 6 lines

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

What activities ? CPU PM notifiers ?

Quoted 66 lines · Lorenzo Pieralisi

Quoted 65 lines · Lina Iyer

Quoted 48 lines · Lorenzo Pieralisi

Quoted 47 lines · Rob Herring

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Quoted 8 lines

Quoted 6 lines

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

What activities ? CPU PM notifiers ?

For today, yes.

However, you can think of CPU PM notifiers as the equivalent of runtime PM hooks. They're called when the "devices" are about to be powered off (runtime suspended) or powered on (runtime resumed.)

However the CPU PM framework and notifiers are rather dumb compared to runtime PM. For example, runtime PM gives you usecounting, autosuspend, control from userspace, statistics, etc. etc. Also, IMO, CPU PM will not scale well for multiple clusters.

What if instead, we used runtime PM for the things that the CPU PM notifiers manager (GIC, VFP, Coresight, etc.), and those drivers used runtime PM callbacks to replace their CPU PM notifiers? We'd then be in a beautiful land where CPU "devices" (and the other connected logic) can be modeled as devices using runtime PM just like every other device in the system.

Then take it up a level... what if we then could use genpd to model the "cluster", made of of the CPUs and "connected" devices (GIC, VFP, etc.) but also modeled the shared L2$ as a device which was using runtime PM.

Now we're in a place where we can use all the benefits of runtime PM, plus the governor features of genpd to start doing a real, multi-CPU, multi-cluster CPUidle that's flexible enough to model the various dependencies in an SoC independent way, but generic enough to be able to use common governors for last-man standing, cache flushing, etc. etc.

Quoted 66 lines · Lorenzo Pieralisi

Quoted 65 lines · Lina Iyer

Quoted 48 lines · Lorenzo Pieralisi

Quoted 47 lines · Rob Herring

Quoted 16 lines · Lina Iyer

Define and add Generic PM domains (genpd) for ARM CPU clusters. Many new SoCs group CPUs as clusters. Clusters share common resources like GIC, power rail, caches, VFP, Coresight etc. When all CPUs in the cluster are idle, these shared resources may also be put in their idle state.

The idle time between the last CPU entering idle and a CPU resuming execution is an opportunity for these shared resources to be powered down. Generic PM domain provides a framework for defining such power domains and attach devices to the domain. When the devices in the domain are idle at runtime, the domain would also be suspended and resumed before the first of the devices resume execution.

We define a generic PM domain for each cluster and attach CPU devices in the cluster to that PM domain. The DT definitions for the SoC describe this relationship. Genpd callbacks for power_on and power_off can then be used to power up/down the shared resources for the domain.

[...]

Quoted 11 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Quoted 8 lines

Quoted 6 lines

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

What activities ? CPU PM notifiers ?

For today, yes.

However, you can think of CPU PM notifiers as the equivalent of runtime PM hooks. They're called when the "devices" are about to be powered off (runtime suspended) or powered on (runtime resumed.)

However the CPU PM framework and notifiers are rather dumb compared to runtime PM. For example, runtime PM gives you usecounting, autosuspend, control from userspace, statistics, etc. etc. Also, IMO, CPU PM will not scale well for multiple clusters.

What if instead, we used runtime PM for the things that the CPU PM notifiers manager (GIC, VFP, Coresight, etc.), and those drivers used runtime PM callbacks to replace their CPU PM notifiers? We'd then be in a beautiful land where CPU "devices" (and the other connected logic) can be modeled as devices using runtime PM just like every other device in the system.

Then take it up a level... what if we then could use genpd to model the "cluster", made of of the CPUs and "connected" devices (GIC, VFP, etc.) but also modeled the shared L2$ as a device which was using runtime PM.

Now we're in a place where we can use all the benefits of runtime PM, plus the governor features of genpd to start doing a real, multi-CPU, multi-cluster CPUidle that's flexible enough to model the various dependencies in an SoC independent way, but generic enough to be able to use common governors for last-man standing, cache flushing, etc. etc.

On Fri, Aug 14, 2015 at 04:51:15AM +0100, Kevin Hilman wrote:

Quoted 9 lines

However, you can think of CPU PM notifiers as the equivalent of runtime PM hooks. They're called when the "devices" are about to be powered off (runtime suspended) or powered on (runtime resumed.)

However the CPU PM framework and notifiers are rather dumb compared to runtime PM. For example, runtime PM gives you usecounting, autosuspend, control from userspace, statistics, etc. etc. Also, IMO, CPU PM will not scale well for multiple clusters.

Quoted 6 lines

What if instead, we used runtime PM for the things that the CPU PM notifiers manager (GIC, VFP, Coresight, etc.), and those drivers used runtime PM callbacks to replace their CPU PM notifiers? We'd then be in a beautiful land where CPU "devices" (and the other connected logic) can be modeled as devices using runtime PM just like every other device in the system.

I would agree with that (even though I do not see how we can make eg GIC, VFP and arch timers behave like devices from a runtime PM standpoint),

still I do not see why we need a virtual power domain for that, the CPU "devices" should be attached to the HW CPU power domain.

More below for systems relying on FW interfaces to handle CPU power management.

Quoted 3 lines

Then take it up a level... what if we then could use genpd to model the "cluster", made of of the CPUs and "connected" devices (GIC, VFP, etc.) but also modeled the shared L2$ as a device which was using runtime PM.

I have to understand what "modeled" means (do we create a struct device on purpose for that ? Same goes for GIC and VFP).

But overall I get the gist of what you are saying, we just have to see how this can be implemented within the genPD framework.

I suspect the "virtual" power domain you are introducing is there for systems where the power controller is hidden from the kernel (ie PSCI), where basically the CPU "devices" can't be attached to a power domain simply because that power domain is not managed in the kernel but by firmware.

Quoted 5 lines

Now we're in a place where we can use all the benefits of runtime PM, plus the governor features of genpd to start doing a real, multi-CPU, multi-cluster CPUidle that's flexible enough to model the various dependencies in an SoC independent way, but generic enough to be able to use common governors for last-man standing, cache flushing, etc. etc.

I do not disagree (even though I think that last man standing is pushing this concept a bit over the top), I am just concerned about the points raised above, most of them should be reasonably simple to solve.

Quoted 2 lines · Lorenzo Pieralisi

Quoted 1 line · Rob Herring

On Tue, Aug 4, 2015 at 6:35 PM, Lina Iyer <[email protected]> wrote:

Quoted 28 lines

Quoted 28 lines

Quoted 12 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

An analogy to this would be the "arm,idle-state" that defines the DT node as something that also depicts a generic cpuidle C state.

Quoted 3 lines · Lina Iyer

Quoted 2 lines · Lorenzo Pieralisi

Quoted 1 line · Rob Herring

On Tue, Aug 4, 2015 at 6:35 PM, Lina Iyer <[email protected]> wrote:

[..]

Quoted 44 lines

Quoted 28 lines

Quoted 28 lines

Quoted 12 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +11 -0

+ARM CPU Power domains
+
+The device tree allows describing of CPU power domains in a SoC. In ARM SoC,
+CPUs may be grouped as clusters. A cluster may have CPUs, GIC, Coresight,
+caches, VFP and power controller and other peripheral hardware. Generally,
+when the CPUs in the cluster are idle/suspended, the shared resources may also
+be suspended and resumed before any of the CPUs resume execution.
+
+CPUs are the defined as the PM domain consumers and there is a PM domain
+provider for the CPUs. Bindings for generic PM domains (genpd) is described in
+[1].

Documentation/devicetree/bindings/arm/cpudomains.txt line 21

Quoted 10 lines

Documentation/devicetree/bindings/arm/cpudomains.txt new file +10 -0

+
+The ARM CPU PM domain follows the same binding convention as any generic PM
+domain. Additional binding properties are -
+
+- compatible:
+       Usage: required
+       Value type: <string>
+       Definition: Must also have
+                       "arm,pd"
+               inorder to initialize the genpd provider as ARM CPU PM domain.

A compatible string should represent a particular h/w block. If it is generic, it should represent some sort of standard programming interface (e.g, AHCI, EHCI, etc.). This doesn't seem to be either and is rather just a mapping of what "driver" you want to use.

Quoted 8 lines

Quoted 4 lines

I would expect that identifying a cpu's or cluster's power domain would be done by a phandle between the cpu/cluster node and power domain node. But I've not really looked at the power domain bindings so who knows.

I would expect the same, meaning that a cpu node, like any other device node would have a phandle pointing at the respective HW power domain.

CPUs have phandles to their domains. That is how the relationship between the domain provider (power-controller) and the consumer (CPU) is established.

Quoted 4 lines

I do not really understand why we want a "generic" CPU power domain, what purpose does it serve ? Creating a collection of cpu devices that we can call "cluster" ?

Quoted 6 lines

Nope, not for calling a cluster, a cluster :)

This compatible is used to define a generic behavior of the CPU domain controller (in addition to the platform specific behavior of the domain power controller). The kernel activities for such power controller are generally the same which otherwise would be repeated across platforms.

Having gone through this series and the one using it[1], the only common activity is just cluster pm notifiers. Other than that it's just creating indirection for now. The scenario might change in future, but for now it seems unnecessary.

Also if you look at the shmobile power controller driver, it covers all the devices including CPUs unlike QCOM power controller which handles only CPU. Yes we can skip CPU genpd creation there only for CPU, IMO creating the power domains should be part of power controller driver.

You can add helper functions for all the ARM specific code that can be reused by multiple power controller drivers handling CPU/Cluster power domain.

Quoted 3 lines

An analogy to this would be the "arm,idle-state" that defines the DT node as something that also depicts a generic cpuidle C state.

I tend to disagree. In idle-states, the nodes define that generic properties and they can be parsed in a generic way. That's not the case here. Each power controller binding differs.

Yes the generic compatible might be useful to identify that this power domain handles CPU/Cluster, but there will be more power controller specific things compared to generic code.

[1] http://www.spinics.net/lists/arm-kernel/msg437304.html

Quoted 1 line · Sudeep Holla

Quoted 6 lines

Having gone through this series and the one using it[1], the only common activity is just cluster pm notifiers. Other than that it's just creating indirection for now. The scenario might change in future, but for now it seems unnecessary.

Not sure, what seems unnecessary to you. Platforms do have to send cluster PM notifications, and they have to duplicate reference counting. Also PM domain framework allows hierarchy which is quite desirable to power down parts of the SoC that are powered on or have to clocked high, until the CPU is running.

Cluster PM notifications are just one aspect of this that we currently handle in the first submission. The patchset as a whole provides a way to determine in Linux the last man down and the first man up and carry out activities. There are a bunch of things that are done to power save when the last man goes down - Turn off debuggers, switch off PLLs, reduce bus clocks, flush caches amongst a few that I know of. Some of it are platform specific and some of it arent. This patches provide the way for both of them to be done easily. The CPU runtime PM and PM domains as a framework, closely track what the hardware does.

Mentioned in an other mail in this thread, is also an option to determine the cluster flush state and use it in conjunction with PSCI to do OS-Initiated cluster power down.

Quoted 9 lines

Also if you look at the shmobile power controller driver, it covers all the devices including CPUs unlike QCOM power controller which handles only CPU. Yes we can skip CPU genpd creation there only for CPU, IMO creating the power domains should be part of power controller driver.

You can add helper functions for all the ARM specific code that can be reused by multiple power controller drivers handling CPU/Cluster power domain.

Sure, some architectures may desire that. I have them addressed in [2].

Quoted 4 lines

Quoted 3 lines

An analogy to this would be the "arm,idle-state" that defines the DT node as something that also depicts a generic cpuidle C state.

Quoted 4 lines

I tend to disagree. In idle-states, the nodes define that generic properties and they can be parsed in a generic way. That's not the case here. Each power controller binding differs.

Yes, may be we will have common elements like latency, residency of powering on/off a domain that a genpd governor can utilize in determining if its worth powering off the domain or not.

Quoted 4 lines

Yes the generic compatible might be useful to identify that this power domain handles CPU/Cluster, but there will be more power controller specific things compared to generic code.

Agreed, not debating that. Power controller is very SoC specific, but not debuggers, GIC, caches, buses etc. Many SoCs have almost similiar needs for many of these supplemental hardware to the CPUs and trend seems to be generalizing on many of these components.

In addition to the common power up/down actions of CPU PM domain core, platforms may have additional configuration before the CPU domain can be powered off or considered active. Allow platform drivers to register handlers for CPU PM domains.

Platform drivers may register their callbacks against a compatible string defined by their PM domain provider device node in the DT. At domain init, the platform driver can initialize the platform specific genpd attributes. The init callback would need to return successfully, for the platform power_on/off handlers to be registered with the CPU PM domain.

The code uses __init section to reduce memory needed for platform handlers and therefore can be freed after the driver is initialized, a desirable outcome for single kernel image.

arch/arm/include/asm/arm-pd.h new file +18 -0

new file mode 100644
index 0000000..fc44abf
@@ -0,0 +1,30 @@
+/*
+ * linux/arch/arm/include/asm/arm-pd.h
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ARM_PD_H__
+#define __ARM_PD_H__
+
+struct of_arm_pd_ops {
+       int (*init)(struct device_node *dn, struct generic_pm_domain *d);
+       int (*power_on)(struct generic_pm_domain *d);
+       int (*power_off)(struct generic_pm_domain *d);
+};

Quoted 15 lines · Lina Iyer

In addition to the common power up/down actions of CPU PM domain core, platforms may have additional configuration before the CPU domain can be powered off or considered active. Allow platform drivers to register handlers for CPU PM domains.

Platform drivers may register their callbacks against a compatible string defined by their PM domain provider device node in the DT. At domain init, the platform driver can initialize the platform specific genpd attributes. The init callback would need to return successfully, for the platform power_on/off handlers to be registered with the CPU PM domain.

The code uses __init section to reduce memory needed for platform handlers and therefore can be freed after the driver is initialized, a desirable outcome for single kernel image.

[...]

Quoted 29 lines

arch/arm/include/asm/arm-pd.h new file +23 -0

new file mode 100644
index 0000000..fc44abf
@@ -0,0 +1,30 @@
+/*
+ * linux/arch/arm/include/asm/arm-pd.h
+ *
+ * Copyright (C) 2015 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ARM_PD_H__
+#define __ARM_PD_H__
+
+struct of_arm_pd_ops {
+       int (*init)(struct device_node *dn, struct generic_pm_domain *d);
+       int (*power_on)(struct generic_pm_domain *d);
+       int (*power_off)(struct generic_pm_domain *d);
+};
+
+struct of_arm_pd_method {
+       const char *handle;
+       struct of_arm_pd_ops *ops;
+};

arch/arm/include/asm/arm-pd.h line 28

Quoted 5 lines

arch/arm/include/asm/arm-pd.h new file +5 -0

+
+#define ARM_PD_METHOD_OF_DECLARE(_name, _handle, _ops) \
+       static const struct of_arm_pd_method __arm_pd_method_of_table_##_name \
+       __used __section(__arm_pd_method_of_table)              \
+       = { .handle = _handle, .ops = _ops }

AFAICT, you are not using this in this series. You should add it when you have a user.

Ideally, we keep some amount of uniformity across various *_OF_DECLARE which is why we have OF_DECLARE_1 and OF_DECLARE_2. This makes all the sections just arrays of struct of_device_id. Not all users follow this, but most do. So instead of putting the ops in here, platforms can provide a function callback which can then set the ops. Then you also don't need the .init() ops function as the callback function can do any initialization too.

Rob

Quoted 15 lines · Lina Iyer

In addition to the common power up/down actions of CPU PM domain core, platforms may have additional configuration before the CPU domain can be powered off or considered active. Allow platform drivers to register handlers for CPU PM domains.

Platform drivers may register their callbacks against a compatible string defined by their PM domain provider device node in the DT. At domain init, the platform driver can initialize the platform specific genpd attributes. The init callback would need to return successfully, for the platform power_on/off handlers to be registered with the CPU PM domain.

The code uses __init section to reduce memory needed for platform handlers and therefore can be freed after the driver is initialized, a desirable outcome for single kernel image.

[...]

Quoted 3 lines

arch/arm/include/asm/arm-pd.h new file +0 -0

new file mode 100644
index 0000000..fc44abf

arch/arm/include/asm/arm-pd.h line 28

Quoted 4 lines

arch/arm/include/asm/arm-pd.h new file +4 -0

+#define ARM_PD_METHOD_OF_DECLARE(_name, _handle, _ops) \
+       static const struct of_arm_pd_method __arm_pd_method_of_table_##_name \
+       __used __section(__arm_pd_method_of_table)              \
+       = { .handle = _handle, .ops = _ops }

AFAICT, you are not using this in this series. You should add it when you have a user.

Ideally, we keep some amount of uniformity across various *_OF_DECLARE which is why we have OF_DECLARE_1 and OF_DECLARE_2. This makes all the sections just arrays of struct of_device_id. Not all users follow this, but most do. So instead of putting the ops in here, platforms can provide a function callback which can then set the ops. Then you also don't need the .init() ops function as the callback function can do any initialization too.