From: Ulf Hansson <ulf.hansson@linaro.org>
To: Sudeep Holla <sudeep.holla@arm.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
Lina Iyer <ilina@codeaurora.org>,
Linux PM <linux-pm@vger.kernel.org>,
Rob Herring <robh+dt@kernel.org>,
Vincent Guittot <vincent.guittot@linaro.org>,
Stephen Boyd <sboyd@kernel.org>, Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Kevin Hilman <khilman@kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH v2 08/13] cpuidle: psci: Add a helper to attach a CPU to its PM domain
Date: Mon, 18 Nov 2019 13:28:29 +0100 [thread overview]
Message-ID: <CAPDyKFoqOPg_kfVTuENSh-m-cW_6M1H4Ma8=8dgPvK8_w4addw@mail.gmail.com> (raw)
In-Reply-To: <20191115171336.GC27170@bogus>
On Fri, 15 Nov 2019 at 18:13, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Tue, Oct 29, 2019 at 05:44:33PM +0100, Ulf Hansson wrote:
> > Introduce a PSCI DT helper function, psci_dt_attach_cpu(), which takes a
> > CPU number as an in-parameter and tries to attach the CPU's struct device
> > to its corresponding PM domain.
> >
> > Let's makes use of dev_pm_domain_attach_by_name(), as it allows us to
> > specify "psci" as the "name" of the PM domain to attach to. Additionally,
> > let's also prepare the attached device to be power managed via runtime PM.
> >
> > Note that, the implementation of the new helper function is in a new
> > separate c-file, which may seems a bit too much at this point. However,
> > subsequent changes that implements the remaining part of the PM domain
> > support for cpuidle-psci, helps to justify this split.
> >
> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > ---
> >
> > Changes in v2:
> > - Reorder patch to be the first one that starts adding the PM domain
> > support.
> > - Rebased.
> >
> > ---
> > drivers/cpuidle/Makefile | 4 ++-
> > drivers/cpuidle/cpuidle-psci-domain.c | 36 +++++++++++++++++++++++++++
> > drivers/cpuidle/cpuidle-psci.h | 12 +++++++++
> > 3 files changed, 51 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/cpuidle/cpuidle-psci-domain.c
> > create mode 100644 drivers/cpuidle/cpuidle-psci.h
> >
> > diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> > index ee70d5cc5b99..cc8c769d7fa9 100644
> > --- a/drivers/cpuidle/Makefile
> > +++ b/drivers/cpuidle/Makefile
> > @@ -21,7 +21,9 @@ obj-$(CONFIG_ARM_U8500_CPUIDLE) += cpuidle-ux500.o
> > obj-$(CONFIG_ARM_AT91_CPUIDLE) += cpuidle-at91.o
> > obj-$(CONFIG_ARM_EXYNOS_CPUIDLE) += cpuidle-exynos.o
> > obj-$(CONFIG_ARM_CPUIDLE) += cpuidle-arm.o
> > -obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle-psci.o
> > +obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle_psci.o
> > +cpuidle_psci-y := cpuidle-psci.o
> > +cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
>
> This was super confusing for a minute until I noticed the difference
> between _ and - used here. I know such pattern is used in the kernel,
> just that it's difficult to notice on first go :)
>
> >
> > ###############################################################################
> > # MIPS drivers
> > diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
> > new file mode 100644
> > index 000000000000..bc7df4dc0686
> > --- /dev/null
> > +++ b/drivers/cpuidle/cpuidle-psci-domain.c
> > @@ -0,0 +1,36 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PM domains for CPUs via genpd - managed by cpuidle-psci.
> > + *
> > + * Copyright (C) 2019 Linaro Ltd.
> > + * Author: Ulf Hansson <ulf.hansson@linaro.org>
> > + *
> > + */
> > +
> > +#include <linux/cpu.h>
> > +#include <linux/device.h>
> > +#include <linux/kernel.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/psci.h>
> > +
> > +#include "cpuidle-psci.h"
> > +
> > +struct device *psci_dt_attach_cpu(int cpu)
> > +{
> > + struct device *dev;
> > +
> > + /* Currently limit the hierarchical topology to be used in OSI mode. */
> > + if (!psci_has_osi_support())
> > + return NULL;
> > +
> > + dev = dev_pm_domain_attach_by_name(get_cpu_device(cpu), "psci");
> > + if (IS_ERR_OR_NULL(dev))
> > + return dev;
> > +
> > + pm_runtime_irq_safe(dev);
> > + if (cpu_online(cpu))
> > + pm_runtime_get_sync(dev);
>
> I probably have to wait till I see the user of this, but until then I
> assume we have some way to deal with CPU HP machinery for this.
Yes, I discussed this with Lorenzo at LPC as well. I did not include a
patch in the series using a CPU HP, simply because I am targeting to
land the basic support first.
For now, this means that the "cluster" will remain on even if there
are CPUs being put offline.
>
> Other than that, it looks fine. I will get back to this to ack or with
> more questions as I review further.
Great, thanks!
Kind regards
Uffe
next prev parent reply other threads:[~2019-11-18 12:29 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-29 16:44 [PATCH v2 00/13] cpuidle: psci: Support hierarchical CPU arrangement Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 01/13] cpuidle: psci: Align psci_power_state count with idle state count Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 02/13] dt: psci: Update DT bindings to support hierarchical PSCI states Ulf Hansson
2019-11-15 17:12 ` Sudeep Holla
2019-11-19 15:15 ` Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 03/13] firmware: psci: Export functions to manage the OSI mode Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 04/13] of: base: Add of_get_cpu_state_node() to get idle states for a CPU node Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 05/13] cpuidle: dt: Support hierarchical CPU idle states Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 06/13] cpuidle: psci: Simplify OF parsing of CPU idle state nodes Ulf Hansson
2019-11-15 17:13 ` Sudeep Holla
2019-10-29 16:44 ` [PATCH v2 07/13] cpuidle: psci: Support hierarchical CPU idle states Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 08/13] cpuidle: psci: Add a helper to attach a CPU to its PM domain Ulf Hansson
2019-11-07 9:13 ` Niklas Cassel
2019-11-07 10:22 ` Ulf Hansson
2019-11-11 11:36 ` Niklas Cassel
2019-11-15 17:13 ` Sudeep Holla
2019-11-18 12:28 ` Ulf Hansson [this message]
2019-10-29 16:44 ` [PATCH v2 09/13] cpuidle: psci: Attach CPU devices to their PM domains Ulf Hansson
2019-11-15 17:15 ` Sudeep Holla
2019-11-18 12:50 ` Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 10/13] cpuidle: psci: Prepare to use OS initiated suspend mode via " Ulf Hansson
2019-11-15 17:30 ` Sudeep Holla
2019-11-18 13:37 ` Ulf Hansson
2019-11-22 18:26 ` Lorenzo Pieralisi
2019-11-25 10:44 ` Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 11/13] cpuidle: psci: Manage runtime PM in the idle path Ulf Hansson
2019-11-15 17:32 ` Sudeep Holla
2019-10-29 16:44 ` [PATCH v2 12/13] cpuidle: psci: Add support for PM domains by using genpd Ulf Hansson
2019-10-29 16:44 ` [PATCH v2 13/13] arm64: dts: Convert to the hierarchical CPU topology layout for MSM8916 Ulf Hansson
2019-11-11 11:00 ` [PATCH v2 00/13] cpuidle: psci: Support hierarchical CPU arrangement Ulf Hansson
2019-11-11 14:31 ` Sudeep Holla
2019-11-22 8:14 ` Ulf Hansson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPDyKFoqOPg_kfVTuENSh-m-cW_6M1H4Ma8=8dgPvK8_w4addw@mail.gmail.com' \
--to=ulf.hansson@linaro.org \
--cc=Lorenzo.Pieralisi@arm.com \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=daniel.lezcano@linaro.org \
--cc=ilina@codeaurora.org \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--cc=sudeep.holla@arm.com \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).