Thread (55 messages) 55 messages, 4 authors, 2019-10-30

Re: [PATCH 10/13] cpuidle: psci: Add a helper to attach a CPU to its PM domain

From: Sudeep Holla <hidden>
Date: 2019-10-28 07:49:21
Also in: linux-arm-msm, linux-pm

On Mon, Oct 28, 2019 at 08:35:55AM +0100, Ulf Hansson wrote:
On Sun, 27 Oct 2019 at 03:30, Sudeep Holla [off-list ref] wrote:
quoted
On Thu, Oct 24, 2019 at 06:47:43PM +0200, Ulf Hansson wrote:
quoted
On Thu, 24 Oct 2019 at 18:31, Sudeep Holla [off-list ref] wrote:
quoted
On Thu, Oct 10, 2019 at 01:39:34PM +0200, Ulf Hansson wrote:
quoted
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.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/cpuidle/cpuidle-psci-domain.c | 21 +++++++++++++++++++++
 drivers/cpuidle/cpuidle-psci.h        |  6 ++++++
 2 files changed, 27 insertions(+)
diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c
index 3f5143ccc3e0..7429fd7626a1 100644
--- a/drivers/cpuidle/cpuidle-psci-domain.c
+++ b/drivers/cpuidle/cpuidle-psci-domain.c
@@ -9,9 +9,11 @@

 #define pr_fmt(fmt) "CPUidle PSCI: " fmt

+#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 <linux/slab.h>
 #include <linux/string.h>
@@ -279,3 +281,22 @@ static int __init psci_idle_init_domains(void)
      return ret;
 }
 subsys_initcall(psci_idle_init_domains);
+
+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");
This clarifies the need for the fixed name. But why not just go by index 0
as the consumer of these psci power-domains will have only one power domain
entry. Why do we need this name compulsory ?
The idea is to be future proof. If I recall correctly, the CPU node on
some QCOM SoCs may also have "CPR" PM domain specified, thus
"multiple" power-domains could be specified.
I am sure we don't want to mx-n-match any power domain provider with
psci. And also I expect in these above mentioned cases, there won't be any
psci power domains.
quoted
In any case, using "psci" doesn't really hurt, right?
Doesn't but I don't see need for one as only one should exist, as mentioned
above we don't want mix-n-match with psci ever.
Not sure I get your point, sorry.

The CPU could very well be attached to more than one power-domain. Of
course not multiple "PSCI power-domains". One could be an PSCI power
domain and another one could be the QCOM CPR (Core power reduction)
power domain.
And who controls QCOM CPR ? If it's OSPM, this model is broken.
I mean OSPM can vote, but the control *has* to be in PSCI firmware to
change any CPU power state.

If it's firmware controlled, then there's no need to explicitly specify
both to OSPM.
Have a look at these binding, there are already upstream, perhaps that
clarifies this?
Documentation/devicetree/bindings/opp/qcom-nvmem-cpufreq.txt
OK, I will have a look.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help