diff mbox series

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

Message ID 1490005163-28633-3-git-send-email-ulf.hansson@linaro.org
State Accepted
Commit ffaa42e8a40b7f1041e36b022cd28b7c45e2b564
Headers show
Series PM / Domains: Add support for always on PM domains in genpd | expand

Commit Message

Ulf Hansson March 20, 2017, 10:19 a.m. UTC 
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(-)

-- 
2.7.4

Comments

Viresh Kumar March 20, 2017, 11:11 a.m. UTC | #1 
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
Geert Uytterhoeven March 20, 2017, 11:54 a.m. UTC | #2 
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
Bartlomiej Zolnierkiewicz March 20, 2017, 5:10 p.m. UTC | #3 
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
diff mbox series

Patch

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 */