lkml.org 
[lkml]   [2022]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] PM: domains: Prevent power off for parent unless child is in deepest state
From
31.01.2022 14:37, Ulf Hansson пишет:
> A PM domain managed by genpd may support multiple idlestates. During
> genpd_power_off() a genpd governor may be asked to select one of the
> idlestates based upon the dev PM QoS constraints, for example.
>
> However, there is a problem with the behaviour around this in genpd. More
> precisely, a parent-domain is allowed to be powered off, no matter of what
> idlestate that has been selected for the child-domain.
>
> So far, we have not received any reports about errors, possibly because
> there might not be platform with this hierarchical configuration, yet.
> Nevertheless, it seems reasonable to change the behaviour into preventing
> the parent-domain from being powered off, unless the deepest idlestate has
> been selected for the child-domain, so let's do that.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/base/power/domain.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 5db704f02e71..7f97c5cabdc2 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -636,6 +636,17 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> atomic_read(&genpd->sd_count) > 0)
> return -EBUSY;
>
> + /*
> + * The children must be in their deepest states to allow the parent to
> + * be powered off. Note that, there's no need for additional locking, as
> + * powering on a child, requires the parent's lock to be acquired first.
> + */
> + list_for_each_entry(link, &genpd->parent_links, parent_node) {
> + struct generic_pm_domain *child = link->child;
> + if (child->state_idx < child->state_count - 1)
> + return -EBUSY;
> + }
> +
> list_for_each_entry(pdd, &genpd->dev_list, list_node) {
> enum pm_qos_flags_status stat;
>
> @@ -1073,6 +1084,13 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
> || atomic_read(&genpd->sd_count) > 0)
> return;
>
> + /* Check that the children are in their deepest state. */
> + list_for_each_entry(link, &genpd->parent_links, parent_node) {
> + struct generic_pm_domain *child = link->child;
> + if (child->state_idx < child->state_count - 1)
> + return;
> + }
> +
> /* Choose the deepest state when suspending */
> genpd->state_idx = genpd->state_count - 1;
> if (_genpd_power_off(genpd, false))

Hello Ulf,

Is this needed by a concrete SoC? It needs to be clarified in the commit
message, otherwise looks like this patch wasn't tested and it's unclear
whether this change is really needed.

\
 
 \ /
  Last update: 2022-01-31 19:29    [W:0.052 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site