[PATCH 0/4] PM / Domains: Add support for always on PM domains in genpd

[PATCH 0/4] PM / Domains: Add support for always on PM domains in genpd

[Ulf Hansson]

The current method to implement an always on PM domain by users of genpd,
consists of returning -EBUSY from their ->power_off() callback. This approach is
suboptimal as genpd is required to follow the regular execution path of its
power off sequence, which ends by invoking the ->power_off() callback.

To enable genpd to early abort the power off sequence for always on PM domains,
it needs static information about these configurations. Therefore this series
invents add new genpd configuration flag, GENPD_FLAG_ALWAYS_ON, which allow
users to explicity tell genpd about these kind of PM domains.

Some additonal changes, which are related this context, are also folded in as a
part of the series.

Ulf Hansson (4):
  PM / Domains: Clean up code validating genpd's status
  PM / Domains: Enable users of genpd to specify always on PM domains
  PM / Domains: Respect errors from genpd's ->power_off() callback
  PM / Domains: Don't warn about IRQ safe device for an always on PM
    domain

 drivers/base/power/domain.c | 42 ++++++++++++++++++++++++++++--------------
 include/linux/pm_domain.h   |  1 +
 2 files changed, 29 insertions(+), 14 deletions(-)

-- 
2.7.4

[PATCH 1/4] PM / Domains: Clean up code validating genpd's status

[Ulf Hansson]

There exists several similar validations of the genpd->status, against
GPD_STATE_ACTIVE and GPD_STATE_POWER_OFF. Let's clean up this code by
converting to use a helper macro, genpd_status_on().

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e697dec..7a8e70d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -121,6 +121,7 @@ static const struct genpd_lock_ops genpd_spin_ops = {
 #define genpd_lock_interruptible(p)	p->lock_ops->lock_interruptible(p)
 #define genpd_unlock(p)			p->lock_ops->unlock(p)
 
+#define genpd_status_on(genpd)		(genpd->status == GPD_STATE_ACTIVE)
 #define genpd_is_irq_safe(genpd)	(genpd->flags & GENPD_FLAG_IRQ_SAFE)
 
 static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
@@ -296,8 +297,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 	 * (1) The domain is already in the "power off" state.
 	 * (2) System suspend is in progress.
 	 */
-	if (genpd->status == GPD_STATE_POWER_OFF
-	    || genpd->prepared_count > 0)
+	if (!genpd_status_on(genpd) || genpd->prepared_count > 0)
 		return 0;
 
 	if (atomic_read(&genpd->sd_count) > 0)
@@ -373,7 +373,7 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
 	struct gpd_link *link;
 	int ret = 0;
 
-	if (genpd->status == GPD_STATE_ACTIVE)
+	if (genpd_status_on(genpd))
 		return 0;
 
 	/*
@@ -752,7 +752,7 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
 {
 	struct gpd_link *link;
 
-	if (genpd->status == GPD_STATE_POWER_OFF)
+	if (!genpd_status_on(genpd))
 		return;
 
 	if (genpd->suspended_count != genpd->device_count
@@ -793,7 +793,7 @@ static void genpd_sync_power_on(struct generic_pm_domain *genpd, bool use_lock,
 {
 	struct gpd_link *link;
 
-	if (genpd->status == GPD_STATE_ACTIVE)
+	if (genpd_status_on(genpd))
 		return;
 
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
@@ -1329,8 +1329,7 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 	genpd_lock(subdomain);
 	genpd_lock_nested(genpd, SINGLE_DEPTH_NESTING);
 
-	if (genpd->status == GPD_STATE_POWER_OFF
-	    &&  subdomain->status != GPD_STATE_POWER_OFF) {
+	if (!genpd_status_on(genpd) && genpd_status_on(subdomain)) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1346,7 +1345,7 @@ static int genpd_add_subdomain(struct generic_pm_domain *genpd,
 	list_add_tail(&link->master_node, &genpd->master_links);
 	link->slave = subdomain;
 	list_add_tail(&link->slave_node, &subdomain->slave_links);
-	if (subdomain->status != GPD_STATE_POWER_OFF)
+	if (genpd_status_on(subdomain))
 		genpd_sd_counter_inc(genpd);
 
  out:
@@ -1406,7 +1405,7 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 		list_del(&link->master_node);
 		list_del(&link->slave_node);
 		kfree(link);
-		if (subdomain->status != GPD_STATE_POWER_OFF)
+		if (genpd_status_on(subdomain))
 			genpd_sd_counter_dec(genpd);
 
 		ret = 0;
@@ -2221,7 +2220,7 @@ static int pm_genpd_summary_one(struct seq_file *s,
 
 	if (WARN_ON(genpd->status >= ARRAY_SIZE(status_lookup)))
 		goto exit;
-	if (genpd->status == GPD_STATE_POWER_OFF)
+	if (!genpd_status_on(genpd))
 		snprintf(state, sizeof(state), "%s-%u",
 			 status_lookup[genpd->status], genpd->state_idx);
 	else
-- 
2.7.4

[PATCH 2/4] PM / Domains: Enable users of genpd to specify always on PM domains

[Ulf Hansson]

The current way to implement an always on PM domain consists of returning
-EBUSY from the ->power_off() callback. This is a bit different compared to
using the always on genpd governor, which prevents the PM domain from being
powered off via runtime suspend, but not via system suspend.

The approach to return -EBUSY from the ->power_off() callback to support
always on PM domains in genpd is suboptimal. That is because it requires
genpd to follow the regular execution path of the power off sequence, which
ends by invoking the ->power_off() callback.

To enable genpd to early abort the power off sequence for always on PM
domains, it needs static information about these configurations. Therefore
let's add a new genpd configuration flag, GENPD_FLAG_ALWAYS_ON.

Users of the new GENPD_FLAG_ALWAYS_ON flag, are by genpd required to make
sure the PM domain is powered on before calling pm_genpd_init(). Moreover,
users don't need to implement the ->power_off() callback, as genpd doesn't
ever invoke it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 14 ++++++++++++--
 include/linux/pm_domain.h   |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 7a8e70d..e63712d 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -123,6 +123,7 @@ static const struct genpd_lock_ops genpd_spin_ops = {
 
 #define genpd_status_on(genpd)		(genpd->status == GPD_STATE_ACTIVE)
 #define genpd_is_irq_safe(genpd)	(genpd->flags & GENPD_FLAG_IRQ_SAFE)
+#define genpd_is_always_on(genpd)	(genpd->flags & GENPD_FLAG_ALWAYS_ON)
 
 static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
 		struct generic_pm_domain *genpd)
@@ -300,7 +301,12 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 	if (!genpd_status_on(genpd) || genpd->prepared_count > 0)
 		return 0;
 
-	if (atomic_read(&genpd->sd_count) > 0)
+	/*
+	 * Abort power off for the PM domain in the following situations:
+	 * (1) The domain is configured as always on.
+	 * (2) When the domain has a subdomain being powered on.
+	 */
+	if (genpd_is_always_on(genpd) || atomic_read(&genpd->sd_count) > 0)
 		return -EBUSY;
 
 	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
@@ -752,7 +758,7 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
 {
 	struct gpd_link *link;
 
-	if (!genpd_status_on(genpd))
+	if (!genpd_status_on(genpd) || genpd_is_always_on(genpd))
 		return;
 
 	if (genpd->suspended_count != genpd->device_count
@@ -1491,6 +1497,10 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
 		genpd->dev_ops.start = pm_clk_resume;
 	}
 
+	/* Always-on domains must be powered on at initialization. */
+	if (genpd_is_always_on(genpd) && !genpd_status_on(genpd))
+		return -EINVAL;
+
 	/* Use only one "off" state if there were no states declared */
 	if (genpd->state_count == 0) {
 		ret = genpd_set_default_power_state(genpd);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 5339ed5..9b6abe6 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -20,6 +20,7 @@
 /* Defines used for the flags field in the struct generic_pm_domain */
 #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
 #define GENPD_FLAG_IRQ_SAFE	(1U << 1) /* PM domain operates in atomic */
+#define GENPD_FLAG_ALWAYS_ON	(1U << 2) /* PM domain is always powered on */
 
 enum gpd_status {
 	GPD_STATE_ACTIVE = 0,	/* PM domain is active */
-- 
2.7.4

[PATCH 3/4] PM / Domains: Respect errors from genpd's ->power_off() callback

[Ulf Hansson]

The current code in genpd_sync_power_off(), doesn't care about potential
errors being returned from genpd's ->power_off() callback.

Obviously this behaviour could lead to problems, such as incorrectly
setting the genpd's status to GPD_STATE_POWER_OFF, but also to incorrectly
decrease the subdomain count for the masters, which potentially allows them
to be powered off in the next recursive call to genpd_sync_power_off().

Let's fix this behaviour by bailing out when the ->power_off() callback
returns an error code.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e63712d..8a2bfc8 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -767,7 +767,8 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
 
 	/* Choose the deepest state when suspending */
 	genpd->state_idx = genpd->state_count - 1;
-	_genpd_power_off(genpd, false);
+	if (_genpd_power_off(genpd, false))
+		return;
 
 	genpd->status = GPD_STATE_POWER_OFF;
 
-- 
2.7.4

[PATCH 4/4] PM / Domains: Don't warn about IRQ safe device for an always on PM domain

[Ulf Hansson]

When an IRQ safe device is attached to a no sleep domain, genpd prints a
warning once, as to indicate it is a suboptimal configuration from power
consumption point of view.

However the warning doesn't make sense for an always on domain, since it
anyway remains powered on. Therefore, let's change to not print the warning
for this configuration.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/base/power/domain.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 8a2bfc8..bfba02f 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -132,8 +132,12 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
 
 	ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
 
-	/* Warn once if IRQ safe dev in no sleep domain */
-	if (ret)
+	/*
+	 * Warn once if an IRQ safe device is attached to a no sleep domain, as
+	 * to indicate a suboptimal configuration for PM. For an always on
+	 * domain this isn't case, thus don't warn.
+	 */
+	if (ret && !genpd_is_always_on(genpd))
 		dev_warn_once(dev, "PM domain %s will not be powered off\n",
 				genpd->name);
 
-- 
2.7.4

Re: [PATCH 1/4] PM / Domains: Clean up code validating genpd's status

[Viresh Kumar]

On 20-03-17, 11:19, Ulf Hansson wrote:
> There exists several similar validations of the genpd->status, against
> GPD_STATE_ACTIVE and GPD_STATE_POWER_OFF. Let's clean up this code by
> converting to use a helper macro, genpd_status_on().
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 2/4] PM / Domains: Enable users of genpd to specify always on PM domains

[Viresh Kumar]

On 20-03-17, 11:19, Ulf Hansson wrote:
> The current way to implement an always on PM domain consists of returning
> -EBUSY from the ->power_off() callback. This is a bit different compared to
> using the always on genpd governor, which prevents the PM domain from being
> powered off via runtime suspend, but not via system suspend.
> 
> The approach to return -EBUSY from the ->power_off() callback to support
> always on PM domains in genpd is suboptimal. That is because it requires
> genpd to follow the regular execution path of the power off sequence, which
> ends by invoking the ->power_off() callback.
> 
> To enable genpd to early abort the power off sequence for always on PM
> domains, it needs static information about these configurations. Therefore
> let's add a new genpd configuration flag, GENPD_FLAG_ALWAYS_ON.
> 
> Users of the new GENPD_FLAG_ALWAYS_ON flag, are by genpd required to make
> sure the PM domain is powered on before calling pm_genpd_init(). Moreover,
> users don't need to implement the ->power_off() callback, as genpd doesn't
> ever invoke it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 14 ++++++++++++--
>  include/linux/pm_domain.h   |  1 +
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 7a8e70d..e63712d 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -123,6 +123,7 @@ static const struct genpd_lock_ops genpd_spin_ops = {
>  
>  #define genpd_status_on(genpd)		(genpd->status == GPD_STATE_ACTIVE)
>  #define genpd_is_irq_safe(genpd)	(genpd->flags & GENPD_FLAG_IRQ_SAFE)
> +#define genpd_is_always_on(genpd)	(genpd->flags & GENPD_FLAG_ALWAYS_ON)
>  
>  static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
>  		struct generic_pm_domain *genpd)
> @@ -300,7 +301,12 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
>  	if (!genpd_status_on(genpd) || genpd->prepared_count > 0)
>  		return 0;
>  
> -	if (atomic_read(&genpd->sd_count) > 0)
> +	/*
> +	 * Abort power off for the PM domain in the following situations:
> +	 * (1) The domain is configured as always on.
> +	 * (2) When the domain has a subdomain being powered on.
> +	 */
> +	if (genpd_is_always_on(genpd) || atomic_read(&genpd->sd_count) > 0)
>  		return -EBUSY;
>  
>  	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
> @@ -752,7 +758,7 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
>  {
>  	struct gpd_link *link;
>  
> -	if (!genpd_status_on(genpd))
> +	if (!genpd_status_on(genpd) || genpd_is_always_on(genpd))

If we want to have similar code in all the places then maybe we can
write this as:

	if (genpd_is_always_on(genpd) || !genpd_status_on(genpd))

>  		return;
>  
>  	if (genpd->suspended_count != genpd->device_count
> @@ -1491,6 +1497,10 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>  		genpd->dev_ops.start = pm_clk_resume;
>  	}
>  
> +	/* Always-on domains must be powered on at initialization. */
> +	if (genpd_is_always_on(genpd) && !genpd_status_on(genpd))
> +		return -EINVAL;
> +
>  	/* Use only one "off" state if there were no states declared */
>  	if (genpd->state_count == 0) {
>  		ret = genpd_set_default_power_state(genpd);
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 5339ed5..9b6abe6 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -20,6 +20,7 @@
>  /* Defines used for the flags field in the struct generic_pm_domain */
>  #define GENPD_FLAG_PM_CLK	(1U << 0) /* PM domain uses PM clk */
>  #define GENPD_FLAG_IRQ_SAFE	(1U << 1) /* PM domain operates in atomic */
> +#define GENPD_FLAG_ALWAYS_ON	(1U << 2) /* PM domain is always powered on */
>  
>  enum gpd_status {
>  	GPD_STATE_ACTIVE = 0,	/* PM domain is active */

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 3/4] PM / Domains: Respect errors from genpd's ->power_off() callback

[Viresh Kumar]

On 20-03-17, 11:19, Ulf Hansson wrote:
> The current code in genpd_sync_power_off(), doesn't care about potential
> errors being returned from genpd's ->power_off() callback.
> 
> Obviously this behaviour could lead to problems, such as incorrectly
> setting the genpd's status to GPD_STATE_POWER_OFF, but also to incorrectly
> decrease the subdomain count for the masters, which potentially allows them
> to be powered off in the next recursive call to genpd_sync_power_off().
> 
> Let's fix this behaviour by bailing out when the ->power_off() callback
> returns an error code.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e63712d..8a2bfc8 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -767,7 +767,8 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
>  
>  	/* Choose the deepest state when suspending */
>  	genpd->state_idx = genpd->state_count - 1;
> -	_genpd_power_off(genpd, false);
> +	if (_genpd_power_off(genpd, false))
> +		return;
>  
>  	genpd->status = GPD_STATE_POWER_OFF;

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 4/4] PM / Domains: Don't warn about IRQ safe device for an always on PM domain

[Viresh Kumar]

On 20-03-17, 11:19, Ulf Hansson wrote:
> When an IRQ safe device is attached to a no sleep domain, genpd prints a
> warning once, as to indicate it is a suboptimal configuration from power
> consumption point of view.
> 
> However the warning doesn't make sense for an always on domain, since it
> anyway remains powered on. Therefore, let's change to not print the warning
> for this configuration.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/base/power/domain.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 8a2bfc8..bfba02f 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -132,8 +132,12 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev,
>  
>  	ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd);
>  
> -	/* Warn once if IRQ safe dev in no sleep domain */
> -	if (ret)
> +	/*
> +	 * Warn once if an IRQ safe device is attached to a no sleep domain, as
> +	 * to indicate a suboptimal configuration for PM. For an always on
> +	 * domain this isn't case, thus don't warn.
> +	 */
> +	if (ret && !genpd_is_always_on(genpd))
>  		dev_warn_once(dev, "PM domain %s will not be powered off\n",
>  				genpd->name);

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 1/4] PM / Domains: Clean up code validating genpd's status

[Geert Uytterhoeven]

On Mon, Mar 20, 2017 at 11:19 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> There exists several similar validations of the genpd->status, against
> GPD_STATE_ACTIVE and GPD_STATE_POWER_OFF. Let's clean up this code by
> converting to use a helper macro, genpd_status_on().
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/4] PM / Domains: Enable users of genpd to specify always on PM domains

[Geert Uytterhoeven]

On Mon, Mar 20, 2017 at 11:19 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The current way to implement an always on PM domain consists of returning
> -EBUSY from the ->power_off() callback. This is a bit different compared to
> using the always on genpd governor, which prevents the PM domain from being
> powered off via runtime suspend, but not via system suspend.
>
> The approach to return -EBUSY from the ->power_off() callback to support
> always on PM domains in genpd is suboptimal. That is because it requires
> genpd to follow the regular execution path of the power off sequence, which
> ends by invoking the ->power_off() callback.
>
> To enable genpd to early abort the power off sequence for always on PM
> domains, it needs static information about these configurations. Therefore
> let's add a new genpd configuration flag, GENPD_FLAG_ALWAYS_ON.
>
> Users of the new GENPD_FLAG_ALWAYS_ON flag, are by genpd required to make
> sure the PM domain is powered on before calling pm_genpd_init(). Moreover,
> users don't need to implement the ->power_off() callback, as genpd doesn't
> ever invoke it.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/4] PM / Domains: Respect errors from genpd's ->power_off() callback

[Geert Uytterhoeven]

On Mon, Mar 20, 2017 at 11:19 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> The current code in genpd_sync_power_off(), doesn't care about potential
> errors being returned from genpd's ->power_off() callback.
>
> Obviously this behaviour could lead to problems, such as incorrectly
> setting the genpd's status to GPD_STATE_POWER_OFF, but also to incorrectly
> decrease the subdomain count for the masters, which potentially allows them
> to be powered off in the next recursive call to genpd_sync_power_off().
>
> Let's fix this behaviour by bailing out when the ->power_off() callback
> returns an error code.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 4/4] PM / Domains: Don't warn about IRQ safe device for an always on PM domain

[Geert Uytterhoeven]

On Mon, Mar 20, 2017 at 11:19 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> When an IRQ safe device is attached to a no sleep domain, genpd prints a
> warning once, as to indicate it is a suboptimal configuration from power
> consumption point of view.
>
> However the warning doesn't make sense for an always on domain, since it
> anyway remains powered on. Therefore, let's change to not print the warning
> for this configuration.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/4] PM / Domains: Clean up code validating genpd's status

[Bartlomiej Zolnierkiewicz]

On Monday, March 20, 2017 11:19:20 AM Ulf Hansson wrote:
> There exists several similar validations of the genpd->status, against
> GPD_STATE_ACTIVE and GPD_STATE_POWER_OFF. Let's clean up this code by
> converting to use a helper macro, genpd_status_on().
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 2/4] PM / Domains: Enable users of genpd to specify always on PM domains

[Bartlomiej Zolnierkiewicz]

On Monday, March 20, 2017 11:19:21 AM Ulf Hansson wrote:
> The current way to implement an always on PM domain consists of returning
> -EBUSY from the ->power_off() callback. This is a bit different compared to
> using the always on genpd governor, which prevents the PM domain from being
> powered off via runtime suspend, but not via system suspend.
> 
> The approach to return -EBUSY from the ->power_off() callback to support
> always on PM domains in genpd is suboptimal. That is because it requires
> genpd to follow the regular execution path of the power off sequence, which
> ends by invoking the ->power_off() callback.
> 
> To enable genpd to early abort the power off sequence for always on PM
> domains, it needs static information about these configurations. Therefore
> let's add a new genpd configuration flag, GENPD_FLAG_ALWAYS_ON.
> 
> Users of the new GENPD_FLAG_ALWAYS_ON flag, are by genpd required to make
> sure the PM domain is powered on before calling pm_genpd_init(). Moreover,
> users don't need to implement the ->power_off() callback, as genpd doesn't
> ever invoke it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 3/4] PM / Domains: Respect errors from genpd's ->power_off() callback

[Bartlomiej Zolnierkiewicz]

On Monday, March 20, 2017 11:19:22 AM Ulf Hansson wrote:
> The current code in genpd_sync_power_off(), doesn't care about potential
> errors being returned from genpd's ->power_off() callback.
> 
> Obviously this behaviour could lead to problems, such as incorrectly
> setting the genpd's status to GPD_STATE_POWER_OFF, but also to incorrectly
> decrease the subdomain count for the masters, which potentially allows them
> to be powered off in the next recursive call to genpd_sync_power_off().
> 
> Let's fix this behaviour by bailing out when the ->power_off() callback
> returns an error code.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 4/4] PM / Domains: Don't warn about IRQ safe device for an always on PM domain

[Bartlomiej Zolnierkiewicz]

On Monday, March 20, 2017 11:19:23 AM Ulf Hansson wrote:
> When an IRQ safe device is attached to a no sleep domain, genpd prints a
> warning once, as to indicate it is a suboptimal configuration from power
> consumption point of view.
> 
> However the warning doesn't make sense for an always on domain, since it
> anyway remains powered on. Therefore, let's change to not print the warning
> for this configuration.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 0/4] PM / Domains: Add support for always on PM domains in genpd

[Kevin Hilman]

Ulf Hansson <ulf.hansson@linaro.org> writes:

> The current method to implement an always on PM domain by users of genpd,
> consists of returning -EBUSY from their ->power_off() callback. This approach is
> suboptimal as genpd is required to follow the regular execution path of its
> power off sequence, which ends by invoking the ->power_off() callback.
>
> To enable genpd to early abort the power off sequence for always on PM domains,
> it needs static information about these configurations. Therefore this series
> invents add new genpd configuration flag, GENPD_FLAG_ALWAYS_ON, which allow
> users to explicity tell genpd about these kind of PM domains.
>
> Some additonal changes, which are related this context, are also folded in as a
> part of the series.

Reviewed-by: Kevin Hilman <khilman@baylibre.com>