[PATCH v3 3/8] PM / Domains: Allow domain power states to be read from DT

[Sudeep Holla]

On 24/10/16 17:48, Lina Iyer wrote:
> Hi Sudeep,
>
> On Mon, Oct 24 2016 at 07:39 -0600, Sudeep Holla wrote:
>>
>>
>> On 14/10/16 18:47, Lina Iyer wrote:
>>> 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 <mtitinger+renesas at baylibre.com>
>>> Signed-off-by: Ulf Hansson <ulf.hansson at linaro.org>
>>> Signed-off-by: Lina Iyer <lina.iyer at linaro.org>
>>> ---
>>> 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.

> 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.

> 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.

> 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 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