Messages in this thread | | | Date | Mon, 31 Jan 2022 21:29:06 +0300 | Subject | Re: [PATCH] PM: domains: Prevent power off for parent unless child is in deepest state | From | Dmitry Osipenko <> |
| |
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.
| |