On Mon, Oct 24 2016 at 11:27 -0600, Sudeep Holla wrote:

On 24/10/16 17:48, Lina Iyer wrote:

quoted

Hi Sudeep,

On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:

quoted

On 14/10/16 18:47, Lina Iyer wrote:

quoted

This patch allows domains to define idle states in the DT. SoC's can
define domain idle states in DT using the "domain-idle-states" property
of the domain provider. Add API to read the idle states from DT that can
be set in the genpd object.

This patch is based on the original patch by Marc Titinger.

Signed-off-by: Marc Titinger <redacted>
Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Lina Iyer <redacted>
---
drivers/base/power/domain.c | 94
+++++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h   |  8 ++++
2 files changed, 102 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 37ab7f1..9af75ba 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c

@@ -1916,6 +1916,100 @@ out:
   return ret ? -EPROBE_DEFER : 0;

}
EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
+
+static const struct of_device_id idle_state_match[] = {
+    { .compatible = "arm,idle-state", },
+    { }
+};
+

I still think it's better to have another compatible to serve this
purpose. We don't want to end up creating genpd domains just because
they are "arm,idle-state" compatible IMO ?

I agree you can prevent it checking for OSC mode support in the
firmware. But I want to understand if you have any strong reasons for
avoiding that approach.

Why are you still held up with OSI/PC PSCI modes?

I am just pointing that out to make sure you are not defining these
DT bindings with just QCOM platform and OSC in consideration.

I am thinking about how can it be used/extended in other use-cases.

quoted

I repeat again this
series is not about any of that, it is just about PM domains.

I completely understand that, no argument on that. What I worry is that
if a system(in future) represents this power domains and domain idles
states and doesn't want to create a genpd, do we have to handle it
differently or even the way your CPUidle series would do or can we say
those states need to specify that they are compatible with the new
feature(the one being added in this series) with a new compatible.
I prefer the latter than the former to avoid all possible workarounds
in future.

quoted

PM domains
have idle states and the idle-state description is similar in definition
to arm,idle-state and therefore uses the same compatible. There is no
point re-defining something that already exists in the kernel.

Yes I understand that but "arm,idle-states" were not defined with this
feature in mind and hence I am bit worried if it could cause any issue
especially if deprecate cpu-idle-states and move to this model
completely. I really don't like this mix and hence I am raising the
concern here. I am trying to ease that migration.

quoted

I was able to find the original thread, where we discussed this [1].

I suggest, you read about PM domains and its idle states and understand
this series in the context of PM domains.

Sure, will do that. Thanks for pointing that out. But the concern I am
raising is entirely different. I am asking if this re-use will cause any
issue in future as pointed out above.

I re-iterate that I understand this series is independent of the CPUIdle
and hence asking why not make it completely independent by just adding
the new compatible.

I am *not asking to redefine something completely*. What I am saying is
*just* to add new compatible that may(for cpu devices)/may not(for
other/non-CPU devices) be used along with "arm,idle-state".

I am fine with that, as long as the compatible string is an alternate
for "arm,idle-state" it shouldn't be a problem.

Any recommendations?

Thanks,
Lina

I may be too paranoid here but I think that's safer. It helps to skip
creating of genpd if required for some domains as I had explained before.

-- 
Regards,
Sudeep