[PATCH 2/4] dt-bindings: add domain-idle-state compatible to arm,idle-state

[PATCH 2/4] dt-bindings: add domain-idle-state compatible to arm,idle-state

[Lina Iyer]

CPU's idle states are defined by the arm,idle-state compatible flag. PM
domains that can contains devices and other domains also has similar
definition for its idle state. Reuse the definition of arm,idle-state
for PM domains by allowing an addition compatible string
("domain-idle-state") to denote idle states that are specific to PM
Domains.

Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/devicetree/bindings/arm/idle-states.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/idle-states.txt b/Documentation/devicetree/bindings/arm/idle-states.txt
index b8e41c1..4b0ff59 100644
--- a/Documentation/devicetree/bindings/arm/idle-states.txt
+++ b/Documentation/devicetree/bindings/arm/idle-states.txt
@@ -271,6 +271,9 @@ follows:
 		Usage: Required
 		Value type: <stringlist>
 		Definition: Must be "arm,idle-state".
+			    Additionally, nodes that are used to describe a
+			    idle-state of PM domain must also define
+			    "domain-idle-state" as compatible string.
 
 	- local-timer-stop
 		Usage: See definition
-- 
2.7.4

[PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Lina Iyer]

Update domain-idle-state binding to use "domain-idle-state" compatible
from Documentation/devicetree/bindings/arm/idle-states.txt.

Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Suggested-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..6fb53a3 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -30,8 +30,9 @@ Optional properties:
    available in the next section.
 
 - domain-idle-states : A phandle of an idle-state that shall be soaked into a
-                generic domain power state. The idle state definitions are
-                compatible with arm,idle-state specified in [1].
+                generic domain power state. The idle state definitions must be
+                compatible with "domain-idle-state" as well as
+                "arm,idle-state" as defined in [1].
   The domain-idle-state property reflects the idle state of this PM domain and
   not the idle states of the devices or sub-domains in the PM domain. Devices
   and sub-domains have their own idle-states independent of the parent
@@ -85,7 +86,7 @@ Example 3:
 	};
 
 	DOMAIN_RET: state@0 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state", "arm,idle-state";
 		reg = <0x0>;
 		entry-latency-us = <1000>;
 		exit-latency-us = <2000>;
@@ -93,7 +94,7 @@ Example 3:
 	};
 
 	DOMAIN_PWR_DN: state@1 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state", "arm,idle-state";
 		reg = <0x1>;
 		entry-latency-us = <5000>;
 		exit-latency-us = <8000>;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Sudeep Holla]

On 25/10/16 16:26, Lina Iyer wrote:
> Update domain-idle-state binding to use "domain-idle-state" compatible
> from Documentation/devicetree/bindings/arm/idle-states.txt.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..6fb53a3 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -30,8 +30,9 @@ Optional properties:
>     available in the next section.
>
>  - domain-idle-states : A phandle of an idle-state that shall be soaked into a
> -                generic domain power state. The idle state definitions are
> -                compatible with arm,idle-state specified in [1].
> +                generic domain power state. The idle state definitions must be
> +                compatible with "domain-idle-state"

I would reword the below a bit different so that it's flexible to be
reused without "arm,idle-state".

> as well as
> +                "arm,idle-state" as defined in [1].

'Idle states that are "arm,idle-state" compatible are generally
"domain-idle-state" compatible as well if it's a PM domain.'

or something like that in line with what's in patch 2/4.

That would give us the scope of reuse of "domain-idle-state" in device
for future. Also it aligns with your patch 4/4.

Otherwise, it looks good.

-- 
Regards,
Sudeep

Re: [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Lina Iyer]

On Tue, Oct 25 2016 at 09:59 -0600, Sudeep Holla wrote:
>
>
>On 25/10/16 16:26, Lina Iyer wrote:
>>Update domain-idle-state binding to use "domain-idle-state" compatible
>>from Documentation/devicetree/bindings/arm/idle-states.txt.
>>
>>Cc: <devicetree@vger.kernel.org>
>>Cc: Rob Herring <robh@kernel.org>
>>Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
>>Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>>---
>> Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>>diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>>index e165036..6fb53a3 100644
>>--- a/Documentation/devicetree/bindings/power/power_domain.txt
>>+++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>@@ -30,8 +30,9 @@ Optional properties:
>>    available in the next section.
>>
>> - domain-idle-states : A phandle of an idle-state that shall be soaked into a
>>-                generic domain power state. The idle state definitions are
>>-                compatible with arm,idle-state specified in [1].
>>+                generic domain power state. The idle state definitions must be
>>+                compatible with "domain-idle-state"
>
>I would reword the below a bit different so that it's flexible to be
>reused without "arm,idle-state".
>
>>as well as
>>+                "arm,idle-state" as defined in [1].
>
>'Idle states that are "arm,idle-state" compatible are generally
>"domain-idle-state" compatible as well if it's a PM domain.'
>
I believe we should have both compatible strings. Per [1], any CPU that
follows the idle state compatible *must* have "arm,idle-state" as a
compatible. Since we are re-using the same compatible, its only correct
that we retain what is already spec'd up in [1] and in addition provide
this new compatible.

Thanks,
Lina

>or something like that in line with what's in patch 2/4.
>
>That would give us the scope of reuse of "domain-idle-state" in device
>for future. Also it aligns with your patch 4/4.
>
>Otherwise, it looks good.
>
>-- 
>Regards,
>Sudeep

Re: [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Sudeep Holla]

On 25/10/16 17:24, Lina Iyer wrote:
> On Tue, Oct 25 2016 at 09:59 -0600, Sudeep Holla wrote:
>>
>>
>> On 25/10/16 16:26, Lina Iyer wrote:
>>> Update domain-idle-state binding to use "domain-idle-state" compatible
>>> from Documentation/devicetree/bindings/arm/idle-states.txt.
>>>
>>> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>>> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Suggested-by: Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org>
>>> Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>> ---
>>> Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt
>>> b/Documentation/devicetree/bindings/power/power_domain.txt
>>> index e165036..6fb53a3 100644
>>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>>> @@ -30,8 +30,9 @@ Optional properties:
>>>    available in the next section.
>>>
>>> - domain-idle-states : A phandle of an idle-state that shall be
>>> soaked into a
>>> -                generic domain power state. The idle state
>>> definitions are
>>> -                compatible with arm,idle-state specified in [1].
>>> +                generic domain power state. The idle state
>>> definitions must be
>>> +                compatible with "domain-idle-state"
>>
>> I would reword the below a bit different so that it's flexible to be
>> reused without "arm,idle-state".
>>
>>> as well as
>>> +                "arm,idle-state" as defined in [1].
>>
>> 'Idle states that are "arm,idle-state" compatible are generally
>> "domain-idle-state" compatible as well if it's a PM domain.'
>>
> I believe we should have both compatible strings. Per [1], any CPU that
> follows the idle state compatible *must* have "arm,idle-state" as a
> compatible.

Yes that's implicit for a CPU device. But generic power domain bindings
should not have that explicitly as it *can be* used for non CPU device.

> Since we are re-using the same compatible, its only correct
> that we retain what is already spec'd up in [1] and in addition provide
> this new compatible.
>

Yes [1] applies for *CPUs only* while this applies for *any device* and
*any power domain*, so I would drop *must have* "arm,idle-state" here
to keep this generic based on my understanding on how compatibles work.

-- 
Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Kevin Hilman]

Lina Iyer <lina.iyer@linaro.org> writes:

> Update domain-idle-state binding to use "domain-idle-state" compatible
> from Documentation/devicetree/bindings/arm/idle-states.txt.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

With no current users for this, I don't see the point of adding a
compatible now.

IMO, this should wait and be added with the identified user we can
discuss it then.

Kevin

Re: [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Sudeep Holla]

On 25/10/16 21:49, Kevin Hilman wrote:
> Lina Iyer <lina.iyer@linaro.org> writes:
>
>> Update domain-idle-state binding to use "domain-idle-state" compatible
>> from Documentation/devicetree/bindings/arm/idle-states.txt.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> With no current users for this, I don't see the point of adding a
> compatible now.
>
> IMO, this should wait and be added with the identified user we can
> discuss it then.
>

No, IMO it needs to be used for the proposed SoC idle/genpd solution.

I understand the nodes that are "arm,idle-state" compatible can be used
for this new SoC hierarchical idle management, but it was never defined
for that use originally. So this new feature must be advertised by the 
firmware with the presence of "domain-idle-state".

Yes we might have other ways to detect that but I have already seen that 
broken on the reference platform, so we need alternate/DT way to specify 
that.

Not all existing "arm,idle-state" compatible nodes will be capable of
supporting this new SoC idle feature. It's just better and safer for a
new feature getting added that relies on DT to have a new compatible.

-- 
Regards,
Sudeep

Re: [PATCH 3/4] dt-bindings: Update domain-idle-state binding to use correct compatibles

[Rob Herring]

On Wed, Oct 26, 2016 at 09:57:35AM +0100, Sudeep Holla wrote:
> 
> 
> On 25/10/16 21:49, Kevin Hilman wrote:
> > Lina Iyer <lina.iyer@linaro.org> writes:
> > 
> > > Update domain-idle-state binding to use "domain-idle-state" compatible
> > > from Documentation/devicetree/bindings/arm/idle-states.txt.
> > > 
> > > Cc: <devicetree@vger.kernel.org>
> > > Cc: Rob Herring <robh@kernel.org>
> > > Suggested-by: Sudeep Holla <sudeep.holla@arm.com>
> > > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> > > ---
> > >  Documentation/devicetree/bindings/power/power_domain.txt | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > With no current users for this, I don't see the point of adding a
> > compatible now.
> > 
> > IMO, this should wait and be added with the identified user we can
> > discuss it then.
> > 
> 
> No, IMO it needs to be used for the proposed SoC idle/genpd solution.
> 
> I understand the nodes that are "arm,idle-state" compatible can be used
> for this new SoC hierarchical idle management, but it was never defined
> for that use originally. So this new feature must be advertised by the
> firmware with the presence of "domain-idle-state".
> 
> Yes we might have other ways to detect that but I have already seen that
> broken on the reference platform, so we need alternate/DT way to specify
> that.
> 
> Not all existing "arm,idle-state" compatible nodes will be capable of
> supporting this new SoC idle feature. It's just better and safer for a
> new feature getting added that relies on DT to have a new compatible.

Or perhaps you should describe something new rather than trying to 
graft in what's there. This combination of compatible strings looks a 
bit odd to me. Though, I've not really spent much time thinking about 
this.

Rob