[PATCH] PM / Domains: Fix for domain idle state

[PATCH] PM / Domains: Fix for domain idle state

[Lina Iyer]

Hi Rafael,

This follows the discussions we had at LPC. The decision to use a different
compatible (different from that of the CPU's idle state) resulted in a new idle
state node definition that is specific to PM domains. The patch describes this
new binding.

You have already pulled in my earlier changes for domain idle states into your
linux-next. This patch is intended to be applied on top of that and fixes the
domain idle state and maps it to the new definition.

Thanks,
Lina

Lina Iyer (1):
  PM / Domains: Fix compatible for domain idle state

 .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
 .../devicetree/bindings/power/power_domain.txt     |  8 +++---
 drivers/base/power/domain.c                        |  2 +-
 3 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt

-- 
2.7.4

[PATCH] PM / Domains: Fix compatible for domain idle state

[Lina Iyer]

Re-using idle state definition provided by arm,idle-state for domain
idle states creates a lot of confusion and limits further evolution of
the domain idle definition. To keep things clear and simple, define a
idle states for domain using a new compatible "domain-idle-state".

Fix existing PM domains code to look for the newly defined compatible.

Cc: <devicetree@vger.kernel.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
 .../devicetree/bindings/power/power_domain.txt     |  8 +++---
 drivers/base/power/domain.c                        |  2 +-
 3 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt

diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
new file mode 100644
index 0000000..eefc7ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
@@ -0,0 +1,33 @@
+PM Domain Idle State Node:
+
+A domain idle state node represents the state parameters that will be used to
+select the state when there are no active components in the domain.
+
+The state node has the following parameters -
+
+- compatible:
+	Usage: Required
+	Value type: <string>
+	Definition: Must be "domain-idle-state".
+
+- entry-latency-us
+	Usage: Required
+	Value type: <prop-encoded-array>
+	Definition: u32 value representing worst case latency in
+		    microseconds required to enter the idle state.
+		    The exit-latency-us duration may be guaranteed
+		    only after entry-latency-us has passed.
+
+- exit-latency-us
+	Usage: Required
+	Value type: <prop-encoded-array>
+	Definition: u32 value representing worst case latency
+		    in microseconds required to exit the idle state.
+
+- min-residency-us
+	Usage: Required
+	Value type: <prop-encoded-array>
+	Definition: u32 value representing minimum residency duration
+		    in microseconds after which the idle state will yield
+		    power benefits after overcoming the overhead in entering
+i		    the idle state.
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index e165036..723e1ad 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -31,7 +31,7 @@ Optional properties:
 
 - 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].
+                compatible with domain-idle-state specified 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 +85,7 @@ Example 3:
 	};
 
 	DOMAIN_RET: state@0 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state";
 		reg = <0x0>;
 		entry-latency-us = <1000>;
 		exit-latency-us = <2000>;
@@ -93,7 +93,7 @@ Example 3:
 	};
 
 	DOMAIN_PWR_DN: state@1 {
-		compatible = "arm,idle-state";
+		compatible = "domain-idle-state";
 		reg = <0x1>;
 		entry-latency-us = <5000>;
 		exit-latency-us = <8000>;
@@ -118,4 +118,4 @@ The node above defines a typical PM domain consumer device, which is located
 inside a PM domain with index 0 of a power controller represented by a node
 with the label "power".
 
-[1]. Documentation/devicetree/bindings/arm/idle-states.txt
+[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 661737c..f0bc672 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
 EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
 
 static const struct of_device_id idle_state_match[] = {
-	{ .compatible = "arm,idle-state", },
+	{ .compatible = "domain-idle-state", },
 	{ }
 };
 
-- 
2.7.4

Re: [PATCH] PM / Domains: Fix compatible for domain idle state

[Ulf Hansson]

On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>  drivers/base/power/domain.c                        |  2 +-
>  3 files changed, 38 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> +       Usage: Required
> +       Value type: <string>
> +       Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> +       Usage: Required
> +       Value type: <prop-encoded-array>
> +       Definition: u32 value representing worst case latency in
> +                   microseconds required to enter the idle state.
> +                   The exit-latency-us duration may be guaranteed
> +                   only after entry-latency-us has passed.

As we anyway are going to change this, why not use an u64 and have the
value in ns instead of us?

That should give us better flexibility and I think this would also be
what Rob would recommend, if I remember earlier similar comments from
him.

> +
> +- exit-latency-us
> +       Usage: Required
> +       Value type: <prop-encoded-array>
> +       Definition: u32 value representing worst case latency
> +                   in microseconds required to exit the idle state.

Ditto.

> +
> +- min-residency-us
> +       Usage: Required
> +       Value type: <prop-encoded-array>
> +       Definition: u32 value representing minimum residency duration
> +                   in microseconds after which the idle state will yield
> +                   power benefits after overcoming the overhead in entering
> +i                  the idle state.

Ditto.

> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
>  - 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].
> +                compatible with domain-idle-state specified 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 +85,7 @@ Example 3:
>         };
>
>         DOMAIN_RET: state@0 {
> -               compatible = "arm,idle-state";
> +               compatible = "domain-idle-state";
>                 reg = <0x0>;
>                 entry-latency-us = <1000>;
>                 exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
>         };
>
>         DOMAIN_PWR_DN: state@1 {
> -               compatible = "arm,idle-state";
> +               compatible = "domain-idle-state";
>                 reg = <0x1>;
>                 entry-latency-us = <5000>;
>                 exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ The node above defines a typical PM domain consumer device, which is located
>  inside a PM domain with index 0 of a power controller represented by a node
>  with the label "power".
>
> -[1]. Documentation/devicetree/bindings/arm/idle-states.txt
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
>  static const struct of_device_id idle_state_match[] = {
> -       { .compatible = "arm,idle-state", },
> +       { .compatible = "domain-idle-state", },
>         { }
>  };
>
> --
> 2.7.4
>

Kind regards
Uffe

Re: [PATCH] PM / Domains: Fix compatible for domain idle state

[Rob Herring]

On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > Re-using idle state definition provided by arm,idle-state for domain
> > idle states creates a lot of confusion and limits further evolution of
> > the domain idle definition. To keep things clear and simple, define a
> > idle states for domain using a new compatible "domain-idle-state".
> >
> > Fix existing PM domains code to look for the newly defined compatible.
> >
> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
> >  drivers/base/power/domain.c                        |  2 +-
> >  3 files changed, 38 insertions(+), 5 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >
> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > new file mode 100644
> > index 0000000..eefc7ed
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > @@ -0,0 +1,33 @@
> > +PM Domain Idle State Node:
> > +
> > +A domain idle state node represents the state parameters that will be used to
> > +select the state when there are no active components in the domain.
> > +
> > +The state node has the following parameters -
> > +
> > +- compatible:
> > +       Usage: Required
> > +       Value type: <string>
> > +       Definition: Must be "domain-idle-state".
> > +
> > +- entry-latency-us
> > +       Usage: Required
> > +       Value type: <prop-encoded-array>
> > +       Definition: u32 value representing worst case latency in
> > +                   microseconds required to enter the idle state.
> > +                   The exit-latency-us duration may be guaranteed
> > +                   only after entry-latency-us has passed.
> 
> As we anyway are going to change this, why not use an u64 and have the
> value in ns instead of us?

I can't imagine that you would need more resolution or range. For times 
less than 1us, s/w and register access times are going to dominate the 
time.

Unless there is a real need, I'd keep alignment with the existing 
binding.

Rob
--
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] PM / Domains: Fix compatible for domain idle state

[Ulf Hansson]

On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > > Re-using idle state definition provided by arm,idle-state for domain
> > > idle states creates a lot of confusion and limits further evolution of
> > > the domain idle definition. To keep things clear and simple, define a
> > > idle states for domain using a new compatible "domain-idle-state".
> > >
> > > Fix existing PM domains code to look for the newly defined compatible.
> > >
> > > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > ---
> > >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
> > >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
> > >  drivers/base/power/domain.c                        |  2 +-
> > >  3 files changed, 38 insertions(+), 5 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >
> > > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > > new file mode 100644
> > > index 0000000..eefc7ed
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > > @@ -0,0 +1,33 @@
> > > +PM Domain Idle State Node:
> > > +
> > > +A domain idle state node represents the state parameters that will be used to
> > > +select the state when there are no active components in the domain.
> > > +
> > > +The state node has the following parameters -
> > > +
> > > +- compatible:
> > > +       Usage: Required
> > > +       Value type: <string>
> > > +       Definition: Must be "domain-idle-state".
> > > +
> > > +- entry-latency-us
> > > +       Usage: Required
> > > +       Value type: <prop-encoded-array>
> > > +       Definition: u32 value representing worst case latency in
> > > +                   microseconds required to enter the idle state.
> > > +                   The exit-latency-us duration may be guaranteed
> > > +                   only after entry-latency-us has passed.
> >
> > As we anyway are going to change this, why not use an u64 and have the
> > value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.

Yep.

>
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.

Agree!

Kind regards
Uffe
--
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] PM / Domains: Fix compatible for domain idle state

[Brendan Jackman]

Hi,

On Thu, Nov 03 2016 at 21:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.

This looks good to me, pinging for review from others/queue for merge.

Best,
Brendan

>
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>  drivers/base/power/domain.c                        |  2 +-
>  3 files changed, 38 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> +	Usage: Required
> +	Value type: <string>
> +	Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> +	Usage: Required
> +	Value type: <prop-encoded-array>
> +	Definition: u32 value representing worst case latency in
> +		    microseconds required to enter the idle state.
> +		    The exit-latency-us duration may be guaranteed
> +		    only after entry-latency-us has passed.
> +
> +- exit-latency-us
> +	Usage: Required
> +	Value type: <prop-encoded-array>
> +	Definition: u32 value representing worst case latency
> +		    in microseconds required to exit the idle state.
> +
> +- min-residency-us
> +	Usage: Required
> +	Value type: <prop-encoded-array>
> +	Definition: u32 value representing minimum residency duration
> +		    in microseconds after which the idle state will yield
> +		    power benefits after overcoming the overhead in entering
> +i		    the idle state.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
>  - 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].
> +                compatible with domain-idle-state specified 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 +85,7 @@ Example 3:
>  	};
>
>  	DOMAIN_RET: state@0 {
> -		compatible = "arm,idle-state";
> +		compatible = "domain-idle-state";
>  		reg = <0x0>;
>  		entry-latency-us = <1000>;
>  		exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
>  	};
>
>  	DOMAIN_PWR_DN: state@1 {
> -		compatible = "arm,idle-state";
> +		compatible = "domain-idle-state";
>  		reg = <0x1>;
>  		entry-latency-us = <5000>;
>  		exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ The node above defines a typical PM domain consumer device, which is located
>  inside a PM domain with index 0 of a power controller represented by a node
>  with the label "power".
>
> -[1]. Documentation/devicetree/bindings/arm/idle-states.txt
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
>  static const struct of_device_id idle_state_match[] = {
> -	{ .compatible = "arm,idle-state", },
> +	{ .compatible = "domain-idle-state", },
>  	{ }
>  };
--
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] PM / Domains: Fix compatible for domain idle state

[Rafael J. Wysocki]

On Mon, Nov 21, 2016 at 1:37 PM, Brendan Jackman
<brendan.jackman@arm.com> wrote:
> Hi,
>
> On Thu, Nov 03 2016 at 21:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Re-using idle state definition provided by arm,idle-state for domain
>> idle states creates a lot of confusion and limits further evolution of
>> the domain idle definition. To keep things clear and simple, define a
>> idle states for domain using a new compatible "domain-idle-state".
>>
>> Fix existing PM domains code to look for the newly defined compatible.
>
> This looks good to me, pinging for review from others/queue for merge.

Well, I need an ACK from at least one DT bindings maintainer so that I
can queue it up.

>> Cc: <devicetree@vger.kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>>  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>>  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>>  drivers/base/power/domain.c                        |  2 +-
>>  3 files changed, 38 insertions(+), 5 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>>
>> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> new file mode 100644
>> index 0000000..eefc7ed
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> @@ -0,0 +1,33 @@
>> +PM Domain Idle State Node:
>> +
>> +A domain idle state node represents the state parameters that will be used to
>> +select the state when there are no active components in the domain.
>> +
>> +The state node has the following parameters -
>> +
>> +- compatible:
>> +     Usage: Required
>> +     Value type: <string>
>> +     Definition: Must be "domain-idle-state".
>> +
>> +- entry-latency-us
>> +     Usage: Required
>> +     Value type: <prop-encoded-array>
>> +     Definition: u32 value representing worst case latency in
>> +                 microseconds required to enter the idle state.
>> +                 The exit-latency-us duration may be guaranteed
>> +                 only after entry-latency-us has passed.
>> +
>> +- exit-latency-us
>> +     Usage: Required
>> +     Value type: <prop-encoded-array>
>> +     Definition: u32 value representing worst case latency
>> +                 in microseconds required to exit the idle state.
>> +
>> +- min-residency-us
>> +     Usage: Required
>> +     Value type: <prop-encoded-array>
>> +     Definition: u32 value representing minimum residency duration
>> +                 in microseconds after which the idle state will yield
>> +                 power benefits after overcoming the overhead in entering
>> +i                the idle state.
>> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
>> index e165036..723e1ad 100644
>> --- a/Documentation/devicetree/bindings/power/power_domain.txt
>> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
>> @@ -31,7 +31,7 @@ Optional properties:
>>
>>  - 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].
>> +                compatible with domain-idle-state specified 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 +85,7 @@ Example 3:
>>       };
>>
>>       DOMAIN_RET: state@0 {
>> -             compatible = "arm,idle-state";
>> +             compatible = "domain-idle-state";
>>               reg = <0x0>;
>>               entry-latency-us = <1000>;
>>               exit-latency-us = <2000>;
>> @@ -93,7 +93,7 @@ Example 3:
>>       };
>>
>>       DOMAIN_PWR_DN: state@1 {
>> -             compatible = "arm,idle-state";
>> +             compatible = "domain-idle-state";
>>               reg = <0x1>;
>>               entry-latency-us = <5000>;
>>               exit-latency-us = <8000>;
>> @@ -118,4 +118,4 @@ The node above defines a typical PM domain consumer device, which is located
>>  inside a PM domain with index 0 of a power controller represented by a node
>>  with the label "power".
>>
>> -[1]. Documentation/devicetree/bindings/arm/idle-states.txt
>> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 661737c..f0bc672 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
>>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>>
>>  static const struct of_device_id idle_state_match[] = {
>> -     { .compatible = "arm,idle-state", },
>> +     { .compatible = "domain-idle-state", },
>>       { }
>>  };
> --

Thanks,
Rafael

Re: [PATCH] PM / Domains: Fix compatible for domain idle state

[Ulf Hansson]

On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>
> Cc: <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>  drivers/base/power/domain.c                        |  2 +-
>  3 files changed, 38 insertions(+), 5 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>
> diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> new file mode 100644
> index 0000000..eefc7ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> @@ -0,0 +1,33 @@
> +PM Domain Idle State Node:
> +
> +A domain idle state node represents the state parameters that will be used to
> +select the state when there are no active components in the domain.
> +
> +The state node has the following parameters -
> +
> +- compatible:
> +       Usage: Required
> +       Value type: <string>
> +       Definition: Must be "domain-idle-state".
> +
> +- entry-latency-us
> +       Usage: Required
> +       Value type: <prop-encoded-array>
> +       Definition: u32 value representing worst case latency in
> +                   microseconds required to enter the idle state.
> +                   The exit-latency-us duration may be guaranteed
> +                   only after entry-latency-us has passed.
> +
> +- exit-latency-us
> +       Usage: Required
> +       Value type: <prop-encoded-array>
> +       Definition: u32 value representing worst case latency
> +                   in microseconds required to exit the idle state.
> +
> +- min-residency-us
> +       Usage: Required
> +       Value type: <prop-encoded-array>
> +       Definition: u32 value representing minimum residency duration
> +                   in microseconds after which the idle state will yield
> +                   power benefits after overcoming the overhead in entering
> +i                  the idle state.
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> index e165036..723e1ad 100644
> --- a/Documentation/devicetree/bindings/power/power_domain.txt
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -31,7 +31,7 @@ Optional properties:
>
>  - 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].
> +                compatible with domain-idle-state specified 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 +85,7 @@ Example 3:
>         };
>
>         DOMAIN_RET: state@0 {
> -               compatible = "arm,idle-state";
> +               compatible = "domain-idle-state";
>                 reg = <0x0>;
>                 entry-latency-us = <1000>;
>                 exit-latency-us = <2000>;
> @@ -93,7 +93,7 @@ Example 3:
>         };
>
>         DOMAIN_PWR_DN: state@1 {
> -               compatible = "arm,idle-state";
> +               compatible = "domain-idle-state";
>                 reg = <0x1>;
>                 entry-latency-us = <5000>;
>                 exit-latency-us = <8000>;
> @@ -118,4 +118,4 @@ The node above defines a typical PM domain consumer device, which is located
>  inside a PM domain with index 0 of a power controller represented by a node
>  with the label "power".
>
> -[1]. Documentation/devicetree/bindings/arm/idle-states.txt
> +[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 661737c..f0bc672 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2048,7 +2048,7 @@ int genpd_dev_pm_attach(struct device *dev)
>  EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>
>  static const struct of_device_id idle_state_match[] = {
> -       { .compatible = "arm,idle-state", },
> +       { .compatible = "domain-idle-state", },
>         { }
>  };
>
> --
> 2.7.4
>

Re: [PATCH] PM / Domains: Fix compatible for domain idle state

[Sudeep Holla]

Hi Lina,

On 03/11/16 21:54, Lina Iyer wrote:
> Re-using idle state definition provided by arm,idle-state for domain
> idle states creates a lot of confusion and limits further evolution of
> the domain idle definition. To keep things clear and simple, define a
> idle states for domain using a new compatible "domain-idle-state".
>
> Fix existing PM domains code to look for the newly defined compatible.
>

Thanks for doing this(reluctantly? :-)). Looks good to me.

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

Re: [PATCH] PM / Domains: Fix compatible for domain idle state

[Ulf Hansson]

On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> > Re-using idle state definition provided by arm,idle-state for domain
>> > idle states creates a lot of confusion and limits further evolution of
>> > the domain idle definition. To keep things clear and simple, define a
>> > idle states for domain using a new compatible "domain-idle-state".
>> >
>> > Fix existing PM domains code to look for the newly defined compatible.
>> >
>> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > ---
>> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>> >  drivers/base/power/domain.c                        |  2 +-
>> >  3 files changed, 38 insertions(+), 5 deletions(-)
>> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > new file mode 100644
>> > index 0000000..eefc7ed
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > @@ -0,0 +1,33 @@
>> > +PM Domain Idle State Node:
>> > +
>> > +A domain idle state node represents the state parameters that will be used to
>> > +select the state when there are no active components in the domain.
>> > +
>> > +The state node has the following parameters -
>> > +
>> > +- compatible:
>> > +       Usage: Required
>> > +       Value type: <string>
>> > +       Definition: Must be "domain-idle-state".
>> > +
>> > +- entry-latency-us
>> > +       Usage: Required
>> > +       Value type: <prop-encoded-array>
>> > +       Definition: u32 value representing worst case latency in
>> > +                   microseconds required to enter the idle state.
>> > +                   The exit-latency-us duration may be guaranteed
>> > +                   only after entry-latency-us has passed.
>>
>> As we anyway are going to change this, why not use an u64 and have the
>> value in ns instead of us?
>
> I can't imagine that you would need more resolution or range. For times
> less than 1us, s/w and register access times are going to dominate the
> time.
>
> Unless there is a real need, I'd keep alignment with the existing
> binding.

Rob, are you fine with this? I thought it would be great to get this
in for 4.10 rc1.

Kind regards
Uffe
--
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] PM / Domains: Fix compatible for domain idle state

[Rafael J. Wysocki]

On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
> > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
> >> > Re-using idle state definition provided by arm,idle-state for domain
> >> > idle states creates a lot of confusion and limits further evolution of
> >> > the domain idle definition. To keep things clear and simple, define a
> >> > idle states for domain using a new compatible "domain-idle-state".
> >> >
> >> > Fix existing PM domains code to look for the newly defined compatible.
> >> >
> >> > Cc: <devicetree@vger.kernel.org>
> >> > Cc: Rob Herring <robh@kernel.org>
> >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> >> > ---
> >> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
> >> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
> >> >  drivers/base/power/domain.c                        |  2 +-
> >> >  3 files changed, 38 insertions(+), 5 deletions(-)
> >> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> > new file mode 100644
> >> > index 0000000..eefc7ed
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> >> > @@ -0,0 +1,33 @@
> >> > +PM Domain Idle State Node:
> >> > +
> >> > +A domain idle state node represents the state parameters that will be used to
> >> > +select the state when there are no active components in the domain.
> >> > +
> >> > +The state node has the following parameters -
> >> > +
> >> > +- compatible:
> >> > +       Usage: Required
> >> > +       Value type: <string>
> >> > +       Definition: Must be "domain-idle-state".
> >> > +
> >> > +- entry-latency-us
> >> > +       Usage: Required
> >> > +       Value type: <prop-encoded-array>
> >> > +       Definition: u32 value representing worst case latency in
> >> > +                   microseconds required to enter the idle state.
> >> > +                   The exit-latency-us duration may be guaranteed
> >> > +                   only after entry-latency-us has passed.
> >>
> >> As we anyway are going to change this, why not use an u64 and have the
> >> value in ns instead of us?
> >
> > I can't imagine that you would need more resolution or range. For times
> > less than 1us, s/w and register access times are going to dominate the
> > time.
> >
> > Unless there is a real need, I'd keep alignment with the existing
> > binding.
> 
> Rob, are you fine with this? I thought it would be great to get this
> in for 4.10 rc1.

Rob, any objections here?

Thanks,
Rafael

Re: [PATCH] PM / Domains: Fix compatible for domain idle state

[Rafael J. Wysocki]

On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
> > On 10 November 2016 at 20:58, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> > Re-using idle state definition provided by arm,idle-state for domain
> > >> > idle states creates a lot of confusion and limits further evolution of
> > >> > the domain idle definition. To keep things clear and simple, define a
> > >> > idle states for domain using a new compatible "domain-idle-state".
> > >> >
> > >> > Fix existing PM domains code to look for the newly defined compatible.
> > >> >
> > >> > Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > >> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > >> > Signed-off-by: Lina Iyer <lina.iyer-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > >> > ---
> > >> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
> > >> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
> > >> >  drivers/base/power/domain.c                        |  2 +-
> > >> >  3 files changed, 38 insertions(+), 5 deletions(-)
> > >> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> >
> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > new file mode 100644
> > >> > index 0000000..eefc7ed
> > >> > --- /dev/null
> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
> > >> > @@ -0,0 +1,33 @@
> > >> > +PM Domain Idle State Node:
> > >> > +
> > >> > +A domain idle state node represents the state parameters that will be used to
> > >> > +select the state when there are no active components in the domain.
> > >> > +
> > >> > +The state node has the following parameters -
> > >> > +
> > >> > +- compatible:
> > >> > +       Usage: Required
> > >> > +       Value type: <string>
> > >> > +       Definition: Must be "domain-idle-state".
> > >> > +
> > >> > +- entry-latency-us
> > >> > +       Usage: Required
> > >> > +       Value type: <prop-encoded-array>
> > >> > +       Definition: u32 value representing worst case latency in
> > >> > +                   microseconds required to enter the idle state.
> > >> > +                   The exit-latency-us duration may be guaranteed
> > >> > +                   only after entry-latency-us has passed.
> > >>
> > >> As we anyway are going to change this, why not use an u64 and have the
> > >> value in ns instead of us?
> > >
> > > I can't imagine that you would need more resolution or range. For times
> > > less than 1us, s/w and register access times are going to dominate the
> > > time.
> > >
> > > Unless there is a real need, I'd keep alignment with the existing
> > > binding.
> > 
> > Rob, are you fine with this? I thought it would be great to get this
> > in for 4.10 rc1.
> 
> Rob, any objections here?

Well, no objections, so applied.

Thanks,
Rafael

--
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] PM / Domains: Fix compatible for domain idle state

[Rob Herring]

On Wed, Dec 7, 2016 at 6:13 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, December 01, 2016 10:21:59 PM Rafael J. Wysocki wrote:
>> On Tuesday, November 29, 2016 09:47:03 AM Ulf Hansson wrote:
>> > On 10 November 2016 at 20:58, Rob Herring <robh@kernel.org> wrote:
>> > > On Mon, Nov 07, 2016 at 12:14:28PM +0100, Ulf Hansson wrote:
>> > >> On 3 November 2016 at 22:54, Lina Iyer <lina.iyer@linaro.org> wrote:
>> > >> > Re-using idle state definition provided by arm,idle-state for domain
>> > >> > idle states creates a lot of confusion and limits further evolution of
>> > >> > the domain idle definition. To keep things clear and simple, define a
>> > >> > idle states for domain using a new compatible "domain-idle-state".
>> > >> >
>> > >> > Fix existing PM domains code to look for the newly defined compatible.
>> > >> >
>> > >> > Cc: <devicetree@vger.kernel.org>
>> > >> > Cc: Rob Herring <robh@kernel.org>
>> > >> > Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> > >> > ---
>> > >> >  .../bindings/power/domain-idle-state.txt           | 33 ++++++++++++++++++++++
>> > >> >  .../devicetree/bindings/power/power_domain.txt     |  8 +++---
>> > >> >  drivers/base/power/domain.c                        |  2 +-
>> > >> >  3 files changed, 38 insertions(+), 5 deletions(-)
>> > >> >  create mode 100644 Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> >
>> > >> > diff --git a/Documentation/devicetree/bindings/power/domain-idle-state.txt b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> > new file mode 100644
>> > >> > index 0000000..eefc7ed
>> > >> > --- /dev/null
>> > >> > +++ b/Documentation/devicetree/bindings/power/domain-idle-state.txt
>> > >> > @@ -0,0 +1,33 @@
>> > >> > +PM Domain Idle State Node:
>> > >> > +
>> > >> > +A domain idle state node represents the state parameters that will be used to
>> > >> > +select the state when there are no active components in the domain.
>> > >> > +
>> > >> > +The state node has the following parameters -
>> > >> > +
>> > >> > +- compatible:
>> > >> > +       Usage: Required
>> > >> > +       Value type: <string>
>> > >> > +       Definition: Must be "domain-idle-state".
>> > >> > +
>> > >> > +- entry-latency-us
>> > >> > +       Usage: Required
>> > >> > +       Value type: <prop-encoded-array>
>> > >> > +       Definition: u32 value representing worst case latency in
>> > >> > +                   microseconds required to enter the idle state.
>> > >> > +                   The exit-latency-us duration may be guaranteed
>> > >> > +                   only after entry-latency-us has passed.
>> > >>
>> > >> As we anyway are going to change this, why not use an u64 and have the
>> > >> value in ns instead of us?
>> > >
>> > > I can't imagine that you would need more resolution or range. For times
>> > > less than 1us, s/w and register access times are going to dominate the
>> > > time.
>> > >
>> > > Unless there is a real need, I'd keep alignment with the existing
>> > > binding.
>> >
>> > Rob, are you fine with this? I thought it would be great to get this
>> > in for 4.10 rc1.
>>
>> Rob, any objections here?
>
> Well, no objections, so applied.

Sorry, just found my ack sitting in my drafts. Thought I had sent it.

Rob