Re: [PATCH v5 5/5] Documentation:powerpc: Add device-tree bindings for power-mgt

[Gautham R Shenoy]

Hello Rob,

Thank you very much for your review. I had missed this mail
and found it while looking at the lkml thread while preparing for the
next iteration.

On Fri, Jan 13, 2017 at 10:57:43AM -0600, Rob Herring wrote:
> On Tue, Jan 10, 2017 at 02:37:04PM +0530, Gautham R. Shenoy wrote:
> > From: "Gautham R. Shenoy" 
> > 
> > Document the device-tree bindings defining the the properties under
> > the @power-mgt node in the device tree that describe the idle states
> > for Linux running on baremetal POWER servers.
> > 
> 
> We have "common" idle state bindings. Perhaps some explanation why those 
> can't be used.

Are you referring to
Documentation/devicetree/bindings/power/domain-idle-state.txt ?

On POWER, since POWER8 time, the DVFS states as well as the idle
states were exposed as properties under the common /ibm,opal/power-mgt
node. Since the DVFS states were large in number, creating a separate
node for each one of them didn't seem like a good design. Hence the
DVFS state properties were encoded as property arrays. The same design
was carried forward to the idle-states as well.

Which is why the common idle-state bindings in domain-idle-state.txt
cannot be used since each idle-state is described as a node there.

> 
> > Signed-off-by: Gautham R. Shenoy 
> > ---
> > [v4]-> [v5]: Fixed a couple of typos.
> > 
> >  .../devicetree/bindings/powerpc/opal/power-mgt.txt | 125 
> > +
> >  1 file changed, 125 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt 
> > b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> > new file mode 100644
> > index 000..4967831
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> > @@ -0,0 +1,125 @@
> > +IBM Power-Management Bindings
> > +=
> > +
> > +Linux running on baremetal POWER machines has access to the processor
> > +idle states. The description of these idle states is exposed via the
> > +node @power-mgt in the device-tree by the firmware.
> > +
> > +Definitions:
> > +
> > +Typically each idle state has the following associated properties:
> > +
> > +- name: The name of the idle state as defined by the firmware.
> > +
> > +- flags: indicating some aspects of this idle states such as the
> > + extent of state-loss, whether timebase is stopped on this
> > + idle states and so on. The flag bits are as follows:
> > +
> > +- exit-latency: The latency involved in transitioning the state of the
> > +   CPU from idle to running.
> > +
> > +- target-residency: The minimum time that the CPU needs to reside in
> > +   this idle state in order to accrue power-savings
> > +   benefit.
> > +
> > +Properties
> > +
> > +The following properties provide details about the idle states. These
> > +properties are optional unless mentioned otherwise below.
> 
> -names is optional but everything else seems to require it. It is not 
> clear what the binding looks like if -names is not present.

The firmware could choose not to expose the idle-states to the kernel,
in which case, it would expose any of these properties.

However, if the firmware chooses to expose idle-states, then the 
ibm,cpu-idle-state-names and ibm,cpu-idle-state-flags properties are
required.

I will reword this as follows:

  The following properties provide details about the idle
  states. These properties are exposed as arrays. Each entry in the
  property array provides the value of that property for the idle
  state associated with the array index of that entry.

  If idle-states are defined, then the properties
  "ibm,cpu-idle-state-names" and "ibm,cpu-idle-state-flags" are
  required. The other properties are required unless mentioned
  otherwise. The length of all the property arrays must be the same.


> 
> > +
> > +- ibm,cpu-idle-state-names:
> > +   Array of strings containing the names of the idle states.
> > +
> > +- ibm,cpu-idle-state-flags:
> > +   Array of unsigned 32-bit values containing the values of the
> > +   flags associated with the the aforementioned idle-states. This
> > +   property is required on POWER9 whenever
> > +   ibm,cpu-idle-state-names is defined and the length of this
> > +   property array should be the same as
> > +   ibm,-cpu-idle-state-names.The flag bits are as follows:
> 
> s/ibm,-cpu/ibm,cpu/
> 
> Needs a space after the period.

Thanks for catching this. Will fix it in the next iteration.

> 
> > +   0x0001 /* Decrementer would stop */
> > +   0x0002 /* Needs timebase restore */
> > +   0x1000 /* Restore GPRs like nap */
> > +   0x2000 /* Restore hypervisor resource from PACA pointer */
> > +   0x4000 /* Program PORE to restore PACA pointer */
> > +

Re: [PATCH v5 5/5] Documentation:powerpc: Add device-tree bindings for power-mgt

[Rob Herring]

On Tue, Jan 10, 2017 at 02:37:04PM +0530, Gautham R. Shenoy wrote:
> From: "Gautham R. Shenoy" 
> 
> Document the device-tree bindings defining the the properties under
> the @power-mgt node in the device tree that describe the idle states
> for Linux running on baremetal POWER servers.
> 

We have "common" idle state bindings. Perhaps some explanation why those 
can't be used.

> Signed-off-by: Gautham R. Shenoy 
> ---
> [v4]-> [v5]: Fixed a couple of typos.
> 
>  .../devicetree/bindings/powerpc/opal/power-mgt.txt | 125 
> +
>  1 file changed, 125 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt 
> b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> new file mode 100644
> index 000..4967831
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
> @@ -0,0 +1,125 @@
> +IBM Power-Management Bindings
> +=
> +
> +Linux running on baremetal POWER machines has access to the processor
> +idle states. The description of these idle states is exposed via the
> +node @power-mgt in the device-tree by the firmware.
> +
> +Definitions:
> +
> +Typically each idle state has the following associated properties:
> +
> +- name: The name of the idle state as defined by the firmware.
> +
> +- flags: indicating some aspects of this idle states such as the
> + extent of state-loss, whether timebase is stopped on this
> + idle states and so on. The flag bits are as follows:
> +
> +- exit-latency: The latency involved in transitioning the state of the
> + CPU from idle to running.
> +
> +- target-residency: The minimum time that the CPU needs to reside in
> + this idle state in order to accrue power-savings
> + benefit.
> +
> +Properties
> +
> +The following properties provide details about the idle states. These
> +properties are optional unless mentioned otherwise below.

-names is optional but everything else seems to require it. It is not 
clear what the binding looks like if -names is not present.

> +
> +- ibm,cpu-idle-state-names:
> + Array of strings containing the names of the idle states.
> +
> +- ibm,cpu-idle-state-flags:
> + Array of unsigned 32-bit values containing the values of the
> + flags associated with the the aforementioned idle-states. This
> + property is required on POWER9 whenever
> + ibm,cpu-idle-state-names is defined and the length of this
> + property array should be the same as
> + ibm,-cpu-idle-state-names.The flag bits are as follows:

s/ibm,-cpu/ibm,cpu/

Needs a space after the period.

> + 0x0001 /* Decrementer would stop */
> + 0x0002 /* Needs timebase restore */
> + 0x1000 /* Restore GPRs like nap */
> + 0x2000 /* Restore hypervisor resource from PACA pointer */
> + 0x4000 /* Program PORE to restore PACA pointer */
> + 0x0001 /* This is a nap state */
> + 0x0002 /* This is a fast-sleep state */
> + 0x0004 /* This is a winkle state */
> + 0x0008 /* This is a fast-sleep state which requires a */
> +/* software workaround for restoring the timebase*/
> + 0x0080 /* This state uses SPR PMICR instruction */
> + 0x0010 /* This is a fast stop state */
> + 0x0020 /* This is a deep-stop state */
> +
> +- ibm,cpu-idle-state-latencies-ns:
> + Array of unsigned 32-bit values containing the values of the
> + exit-latencies (in ns) for the idle states in
> + ibm,cpu-idle-state-names. This property is required whenever
> + ibm,cpu-idle-state-names is defined and the length of this
> + property array should be the same as
> + ibm,-cpu-idle-state-names.

Same typo.

> +
> +- ibm,cpu-idle-state-residency-ns:
> + Array of unsigned 32-bit values containing the values of the
> + target-residency (in ns) for the idle states in
> + ibm,cpu-idle-state-names. On POWER8 this is an optional
> + property. If the property is absent, the target residency for
> + the "Nap", "FastSleep" are defined to 1 and 3
> + respectively. On POWER9 this property must be defined if
> + ibm,cpu-idle-state-names is defined and the length should be
> + same as that of ibm,cpu-idle-state-names.
> +
> +- ibm,cpu-idle-state-psscr:
> + Array of unsigned 64-bit values containing the values for the
> + PSSCR for each of the idle states in ibm,cpu-idle-state-names.
> + This property is required on POWER9 whenever
> + ibm,cpu-idle-state-names is defined and the length of this
> + property array should be the same as
> + ibm,cpu-idle-state-names.
> +
> +- 

[PATCH v5 5/5] Documentation:powerpc: Add device-tree bindings for power-mgt

[Gautham R. Shenoy]

From: "Gautham R. Shenoy" 

Document the device-tree bindings defining the the properties under
the @power-mgt node in the device tree that describe the idle states
for Linux running on baremetal POWER servers.

Signed-off-by: Gautham R. Shenoy 
---
[v4]-> [v5]: Fixed a couple of typos.

 .../devicetree/bindings/powerpc/opal/power-mgt.txt | 125 +
 1 file changed, 125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt

diff --git a/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt 
b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
new file mode 100644
index 000..4967831
--- /dev/null
+++ b/Documentation/devicetree/bindings/powerpc/opal/power-mgt.txt
@@ -0,0 +1,125 @@
+IBM Power-Management Bindings
+=
+
+Linux running on baremetal POWER machines has access to the processor
+idle states. The description of these idle states is exposed via the
+node @power-mgt in the device-tree by the firmware.
+
+Definitions:
+
+Typically each idle state has the following associated properties:
+
+- name: The name of the idle state as defined by the firmware.
+
+- flags: indicating some aspects of this idle states such as the
+ extent of state-loss, whether timebase is stopped on this
+ idle states and so on. The flag bits are as follows:
+
+- exit-latency: The latency involved in transitioning the state of the
+   CPU from idle to running.
+
+- target-residency: The minimum time that the CPU needs to reside in
+   this idle state in order to accrue power-savings
+   benefit.
+
+Properties
+
+The following properties provide details about the idle states. These
+properties are optional unless mentioned otherwise below.
+
+- ibm,cpu-idle-state-names:
+   Array of strings containing the names of the idle states.
+
+- ibm,cpu-idle-state-flags:
+   Array of unsigned 32-bit values containing the values of the
+   flags associated with the the aforementioned idle-states. This
+   property is required on POWER9 whenever
+   ibm,cpu-idle-state-names is defined and the length of this
+   property array should be the same as
+   ibm,-cpu-idle-state-names.The flag bits are as follows:
+   0x0001 /* Decrementer would stop */
+   0x0002 /* Needs timebase restore */
+   0x1000 /* Restore GPRs like nap */
+   0x2000 /* Restore hypervisor resource from PACA pointer */
+   0x4000 /* Program PORE to restore PACA pointer */
+   0x0001 /* This is a nap state */
+   0x0002 /* This is a fast-sleep state */
+   0x0004 /* This is a winkle state */
+   0x0008 /* This is a fast-sleep state which requires a */
+  /* software workaround for restoring the timebase*/
+   0x0080 /* This state uses SPR PMICR instruction */
+   0x0010 /* This is a fast stop state */
+   0x0020 /* This is a deep-stop state */
+
+- ibm,cpu-idle-state-latencies-ns:
+   Array of unsigned 32-bit values containing the values of the
+   exit-latencies (in ns) for the idle states in
+   ibm,cpu-idle-state-names. This property is required whenever
+   ibm,cpu-idle-state-names is defined and the length of this
+   property array should be the same as
+   ibm,-cpu-idle-state-names.
+
+- ibm,cpu-idle-state-residency-ns:
+   Array of unsigned 32-bit values containing the values of the
+   target-residency (in ns) for the idle states in
+   ibm,cpu-idle-state-names. On POWER8 this is an optional
+   property. If the property is absent, the target residency for
+   the "Nap", "FastSleep" are defined to 1 and 3
+   respectively. On POWER9 this property must be defined if
+   ibm,cpu-idle-state-names is defined and the length should be
+   same as that of ibm,cpu-idle-state-names.
+
+- ibm,cpu-idle-state-psscr:
+   Array of unsigned 64-bit values containing the values for the
+   PSSCR for each of the idle states in ibm,cpu-idle-state-names.
+   This property is required on POWER9 whenever
+   ibm,cpu-idle-state-names is defined and the length of this
+   property array should be the same as
+   ibm,cpu-idle-state-names.
+
+- ibm,cpu-idle-state-psscr-mask:
+   Array of unsigned 64-bit values containing the masks
+   indicating which psscr fields are set in the corresponding
+   entries of ibm,cpu-idle-state-psscr.  This property is
+   required on POWER9 whenever ibm,cpu-idle-state-names is
+   defined and the length of this property array should be the
+   same as ibm,cpu-idle-state-names.
+
+   Whenever the firmware sets an entry in
+   ibm,cpu-idle-state-psscr-mask