Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200819104057.318230-1-ulf.hansson@linaro.org>
 <20200819104057.318230-4-ulf.hansson@linaro.org> <20200825161602.GE12523@codeaurora.org>
In-Reply-To: <20200825161602.GE12523@codeaurora.org>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Wed, 26 Aug 2020 08:37:35 +0200
Message-ID: <CAPDyKFrMUf4y5kVjr=dW-wf2kqBwcGePf=55U1Ck8O6tp-OuqQ@mail.gmail.com>
Subject: Re: [PATCH 3/3] PM / Domains: Add support for PM domain on/off
 notifiers for genpd
To:     Lina Iyer <ilina@codeaurora.org>
Cc:     "Rafael J . Wysocki" <rjw@rjwysocki.net>,
        Kevin Hilman <khilman@kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Lukasz Luba <lukasz.luba@arm.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Benjamin Gaignard <benjamin.gaignard@st.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 25 Aug 2020 at 18:16, Lina Iyer <ilina@codeaurora.org> wrote:
>
> On Wed, Aug 19 2020 at 04:41 -0600, Ulf Hansson wrote:
> >A device may have specific HW constraints that must be obeyed to, before
> >its corresponding PM domain (genpd) can be powered off - and vice verse at
> >power on. These constraints can't be managed through the regular runtime PM
> >based deployment for a device, because the access pattern for it, isn't
> >always request based. In other words, using the runtime PM callbacks to
> >deal with the constraints doesn't work for these cases.
> >
> >For these reasons, let's instead add a PM domain power on/off notification
> >mechanism to genpd. To add/remove a notifier for a device, the device must
> >already have been attached to the genpd, which also means that it needs to
> >be a part of the PM domain topology.
> >
> >To add/remove a notifier, let's introduce two genpd specific functions:
> > - dev_pm_genpd_add|remove_notifier()
> >
> >Note that, to further clarify when genpd power on/off notifiers may be
> >used, one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT
> >notifiers. In the long run, the genpd power on/off notifiers should be able
> >to replace them, but that requires additional genpd based platform support
> >for the current users.
> >
> >Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >---
> > drivers/base/power/domain.c | 130 ++++++++++++++++++++++++++++++++++--
> > include/linux/pm_domain.h   |  15 +++++
> > 2 files changed, 141 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> >index 4b787e1ff188..9cb85a5e8342 100644
> >--- a/drivers/base/power/domain.c
> >+++ b/drivers/base/power/domain.c
> >@@ -545,13 +545,21 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> >       if (!genpd->gov)
> >               genpd->state_idx = 0;
> >
> >+      /* Notify consumers that we are about to power off. */
> >+      ret = raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_OFF,
> >+                                    NULL);
> >+      if (ret)
> >+              return ret;
> >+
> >       /* Don't power off, if a child domain is waiting to power on. */
> >-      if (atomic_read(&genpd->sd_count) > 0)
> >-              return -EBUSY;
> >+      if (atomic_read(&genpd->sd_count) > 0) {
> >+              ret = -EBUSY;
> >+              goto busy;
> >+      }
> >
> >       ret = _genpd_power_off(genpd, true);
> >       if (ret)
> >-              return ret;
> >+              goto busy;
> >
> >       genpd->status = GENPD_STATE_OFF;
> >       genpd_update_accounting(genpd);
> >@@ -564,6 +572,9 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> >       }
> >
> >       return 0;
> >+busy:
> >+      raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL);
> It would be helpful to abstract these notification related calls into
> functions of their own. So, for CPU PM domains, it would help to add
> RCU_NONIDLE() around the notifiers, allowing the callbacks to add trace
> functions.

Thanks for the suggestion! It makes perfect sense to me - and would
also be consistent with how CPU PM notifiers are managed,

[,,,]

Kind regards
Uffe