* [PATCH RFC 0/2] ARM: DT: add bindings for system suspend @ 2015-01-21 11:35 Sudeep Holla 2015-01-21 11:35 ` [PATCH RFC 1/2] Documentation: arm: define DT " Sudeep Holla 2015-01-21 11:35 ` [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND Sudeep Holla 0 siblings, 2 replies; 18+ messages in thread From: Sudeep Holla @ 2015-01-21 11:35 UTC (permalink / raw) To: linux-arm-kernel Hi, There are various attempts on the list to implement system suspend on ARM64 using CPU Idle. Here is an attempt to standardise it by defining DT bindings for the same and hence provide a generic solution without associating it with CPUIdle. Currently only PSCI is supported as entry method in this series. Regards, Sudeep P.S: I am on vacation till Jan 31st and will have limited access to emails, so there might be delays in my response. Sudeep Holla (2): Documentation: arm: define DT bindings for system suspend ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND Documentation/devicetree/bindings/arm/psci.txt | 11 +++ .../devicetree/bindings/arm/system-suspend.txt | 93 ++++++++++++++++++++++ arch/arm64/kernel/psci.c | 83 ++++++++++++++++--- 3 files changed, 178 insertions(+), 9 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/system-suspend.txt -- 1.9.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-21 11:35 [PATCH RFC 0/2] ARM: DT: add bindings for system suspend Sudeep Holla @ 2015-01-21 11:35 ` Sudeep Holla 2015-01-21 13:21 ` Jisheng Zhang 2015-02-04 16:10 ` Mark Rutland 2015-01-21 11:35 ` [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND Sudeep Holla 1 sibling, 2 replies; 18+ messages in thread From: Sudeep Holla @ 2015-01-21 11:35 UTC (permalink / raw) To: linux-arm-kernel ARM based platforms implement unique ways to enter system suspend (i.e. Suspend to RAM). The mechanism and the parameters defining the system state vary on a per-platform basis forcing the OS to handle it in very platform specific way. Since ARM 32-bit systems had machine specific code, no attempts to standardize are being made as it provides easy way to implement suspend operations in a platform specific manner. However, this approach not only makes maintainance more difficult as the number of platforms supported increases but also not feasible for ARM64. This DT binding aims at standardizing the system suspend for ARM platforms. ARM64 platforms mandates entry-method property in DT for this system suspend node. On system implementing PSCI as an enable-method to enter system suspend, the PSCI CPU suspend method is used on versions upto v0.2 and requires the power_state parameter to be passed to the PSCI CPU suspend function. This parameter is platform specific, therefore must be provided by firmware to the OS in order to enable proper call sequence. This ARM system suspend DT bindings rely on a property (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes how the PSCI CPU suspend power_state parameter should be defined in DT. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- Documentation/devicetree/bindings/arm/psci.txt | 11 +++ .../devicetree/bindings/arm/system-suspend.txt | 93 ++++++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/system-suspend.txt diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt index 5aa40ede0e99..bd3977a2a333 100644 --- a/Documentation/devicetree/bindings/arm/psci.txt +++ b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ Device tree nodes that require usage of PSCI CPU_SUSPEND function (ie idle Definition: power_state parameter to pass to the PSCI suspend call. +PSCI v0.2 and earlier versions don't have explicit operation for system +suspend. However, one can implement system suspend using CPU_SUSPEND by +ensuring every other core except the one executing the CPU_SUSPEND call +has called into PSCI through a CPU_OFF call. + +In such cases, device tree nodes representing system suspend as per the +bindings in [2] must specify the above "arm,psci-suspend-param" property. + Example: Case 1: PSCI v0.1 only. @@ -100,3 +108,6 @@ Case 3: PSCI v0.2 and PSCI v0.1. [1] Kernel documentation - ARM idle states bindings Documentation/devicetree/bindings/arm/idle-states.txt + +[2] Kernel documentation - ARM system suspend bindings + Documentation/devicetree/bindings/arm/system-suspend.txt diff --git a/Documentation/devicetree/bindings/arm/system-suspend.txt b/Documentation/devicetree/bindings/arm/system-suspend.txt new file mode 100644 index 000000000000..15cac4c7fe92 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/system-suspend.txt @@ -0,0 +1,93 @@ +========================================== +ARM system suspend binding description +========================================== + +========================================== +1 - Introduction +========================================== + +System Suspend(commonly known as Suspend to RAM) is a method to remove +power from most parts of the machine aside from the RAM, which is required +to restore the machine's state. Because of the large power savings, it is +widely used in mobile systems like laptops, tablets and smartphones. + +Usually most mobile systems enter system suspend state aggressively when +they are idle even for short time(in seconds) while others systems like +laptops automatically enter this mode when they running on batteries and +the lid is closed (and/or the user is inactive for some time(in minutes)). + +Most of the devices in the system are deactivated. Non-volatile memory +(like disk drives, flash, memory card), graphics chips and even the CPU +are usually deactivated. Only the RAM is powered to keep its contents. On +resume, only those individual devices/CPUs need to be reinitialized and +work continues relatively fast. + +It is highly hardware specific especially on ARM platforms. Hence we need +device tree binding definition for ARM system suspend state which is the +subject of this document in order to provide generic solution. + +=========================================== +2 - system suspend node +=========================================== + +The system suspend node represents the description of the mechanism to +enter system suspend state and must be defined as follows: + + - compatible + Usage: Required + Value type: <stringlist> + Definition: Must be "arm,system-suspend"; + + - entry-method + Value type: <stringlist> + Usage and definition depend on ARM architecture version. + # On ARM v8 64-bit this property is required and must + be one of: + - "arm,psci" (see bindings in [1]) + + - status: + Usage: Optional + Value type: <string> + Definition: A standard device tree property [2] that indicates + the operational status of system suspend. + If present, it shall be: + "okay": to indicate it is operational. + "disabled": to indicate that it has been disabled + in firmware so it is not operational. + By default, it's always enabled if not explicitly + disabled. + + In addition to the properties listed above, it may require additional + properties specifics to the entry-method defined, please refer to the + corresponding entry-method bindings documentation for details. + In the below example using "arm,psci" entry method, + "arm,psci-suspend-param" is a PSCI specific property. + + The system suspend node's parent node must be the 'cpus' node. + +=========================================== +3 - Examples +=========================================== + +Example: +cpus { + /* cpu-map, cpu and idle-states nodes */ + .... + + system-suspend { + compatible = "arm,system-suspend"; + entry-method = "arm,psci"; + arm,psci-suspend-param = <0x1010000>; + }; + + .... +}; +=========================================== +4 - References +=========================================== + +[1] ARM Linux Kernel documentation - PSCI bindings + Documentation/devicetree/bindings/arm/psci.txt + +[2] ePAPR standard + https://www.power.org/documentation/epapr-version-1-1/ -- 1.9.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-21 11:35 ` [PATCH RFC 1/2] Documentation: arm: define DT " Sudeep Holla @ 2015-01-21 13:21 ` Jisheng Zhang 2015-01-21 13:35 ` Jisheng Zhang 2015-02-04 16:10 ` Mark Rutland 1 sibling, 1 reply; 18+ messages in thread From: Jisheng Zhang @ 2015-01-21 13:21 UTC (permalink / raw) To: linux-arm-kernel Dear Sudeep, On Wed, 21 Jan 2015 03:35:54 -0800 Sudeep Holla <sudeep.holla@arm.com> wrote: > ARM based platforms implement unique ways to enter system suspend > (i.e. Suspend to RAM). The mechanism and the parameters defining the > system state vary on a per-platform basis forcing the OS to handle it > in very platform specific way. > > Since ARM 32-bit systems had machine specific code, no attempts to > standardize are being made as it provides easy way to implement suspend > operations in a platform specific manner. However, this approach not > only makes maintainance more difficult as the number of platforms > supported increases but also not feasible for ARM64. > > This DT binding aims at standardizing the system suspend for ARM > platforms. ARM64 platforms mandates entry-method property in DT for > this system suspend node. > > On system implementing PSCI as an enable-method to enter system suspend, > the PSCI CPU suspend method is used on versions upto v0.2 and requires > the power_state parameter to be passed to the PSCI CPU suspend function. > > This parameter is platform specific, therefore must be provided by > firmware to the OS in order to enable proper call sequence. > > This ARM system suspend DT bindings rely on a property > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > how the PSCI CPU suspend power_state parameter should be defined in DT. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > .../devicetree/bindings/arm/system-suspend.txt | 93 > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/system-suspend.txt > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > b/Documentation/devicetree/bindings/arm/psci.txt index > 5aa40ede0e99..bd3977a2a333 100644 --- > a/Documentation/devicetree/bindings/arm/psci.txt +++ > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ Device > tree nodes that require usage of PSCI CPU_SUSPEND function (ie idle > Definition: power_state parameter to pass to the PSCI suspend call. > > +PSCI v0.2 and earlier versions don't have explicit operation for system > +suspend. However, one can implement system suspend using CPU_SUSPEND by > +ensuring every other core except the one executing the CPU_SUSPEND call > +has called into PSCI through a CPU_OFF call. If users explicitly hot-unplug other cores when system load is low to save power, then we want to suspend at some point, how does the firmware know this case? In my private tree, I extend the PSCI spec to tell firmware we want to suspend the system. Thanks, Jisheng ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-21 13:21 ` Jisheng Zhang @ 2015-01-21 13:35 ` Jisheng Zhang 2015-01-21 13:56 ` Lorenzo Pieralisi 0 siblings, 1 reply; 18+ messages in thread From: Jisheng Zhang @ 2015-01-21 13:35 UTC (permalink / raw) To: linux-arm-kernel Dear Sudeep, On Wed, 21 Jan 2015 05:21:39 -0800 Jisheng Zhang <jszhang@marvell.com> wrote: > Dear Sudeep, > > On Wed, 21 Jan 2015 03:35:54 -0800 > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > ARM based platforms implement unique ways to enter system suspend > > (i.e. Suspend to RAM). The mechanism and the parameters defining the > > system state vary on a per-platform basis forcing the OS to handle it > > in very platform specific way. > > > > Since ARM 32-bit systems had machine specific code, no attempts to > > standardize are being made as it provides easy way to implement suspend > > operations in a platform specific manner. However, this approach not > > only makes maintainance more difficult as the number of platforms > > supported increases but also not feasible for ARM64. > > > > This DT binding aims at standardizing the system suspend for ARM > > platforms. ARM64 platforms mandates entry-method property in DT for > > this system suspend node. > > > > On system implementing PSCI as an enable-method to enter system suspend, > > the PSCI CPU suspend method is used on versions upto v0.2 and requires > > the power_state parameter to be passed to the PSCI CPU suspend function. > > > > This parameter is platform specific, therefore must be provided by > > firmware to the OS in order to enable proper call sequence. > > > > This ARM system suspend DT bindings rely on a property > > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > > how the PSCI CPU suspend power_state parameter should be defined in DT. > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > --- > > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > > .../devicetree/bindings/arm/system-suspend.txt | 93 > > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/arm/system-suspend.txt > > > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > > b/Documentation/devicetree/bindings/arm/psci.txt index > > 5aa40ede0e99..bd3977a2a333 100644 --- > > a/Documentation/devicetree/bindings/arm/psci.txt +++ > > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ Device > > tree nodes that require usage of PSCI CPU_SUSPEND function (ie idle > > Definition: power_state parameter to pass to the PSCI suspend call. > > > > +PSCI v0.2 and earlier versions don't have explicit operation for system > > +suspend. However, one can implement system suspend using CPU_SUSPEND by > > +ensuring every other core except the one executing the CPU_SUSPEND call > > +has called into PSCI through a CPU_OFF call. > > If users explicitly hot-unplug other cores when system load is low to save > power, then we want to suspend at some point, how does the firmware know > this case? Sorry for confusion. I mean If users explicitly hot-unplug other cores when system load is low to save power, then at some point cpuidle want to suspend the cluster, how does the distinguish this case with suspend the system to ram. > > In my private tree, I extend the PSCI spec to tell firmware we want to > suspend the system. I extend the PSCI suspend function to help firmware distinguish suspend system to ram and other suspend cores/clusters. > > Thanks, > Jisheng > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-21 13:35 ` Jisheng Zhang @ 2015-01-21 13:56 ` Lorenzo Pieralisi 2015-01-22 4:33 ` Jisheng Zhang 0 siblings, 1 reply; 18+ messages in thread From: Lorenzo Pieralisi @ 2015-01-21 13:56 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 21, 2015 at 01:35:07PM +0000, Jisheng Zhang wrote: > Dear Sudeep, > > On Wed, 21 Jan 2015 05:21:39 -0800 > Jisheng Zhang <jszhang@marvell.com> wrote: > > > Dear Sudeep, > > > > On Wed, 21 Jan 2015 03:35:54 -0800 > > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > ARM based platforms implement unique ways to enter system suspend > > > (i.e. Suspend to RAM). The mechanism and the parameters defining the > > > system state vary on a per-platform basis forcing the OS to handle it > > > in very platform specific way. > > > > > > Since ARM 32-bit systems had machine specific code, no attempts to > > > standardize are being made as it provides easy way to implement suspend > > > operations in a platform specific manner. However, this approach not > > > only makes maintainance more difficult as the number of platforms > > > supported increases but also not feasible for ARM64. > > > > > > This DT binding aims at standardizing the system suspend for ARM > > > platforms. ARM64 platforms mandates entry-method property in DT for > > > this system suspend node. > > > > > > On system implementing PSCI as an enable-method to enter system suspend, > > > the PSCI CPU suspend method is used on versions upto v0.2 and requires > > > the power_state parameter to be passed to the PSCI CPU suspend function. > > > > > > This parameter is platform specific, therefore must be provided by > > > firmware to the OS in order to enable proper call sequence. > > > > > > This ARM system suspend DT bindings rely on a property > > > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > > > how the PSCI CPU suspend power_state parameter should be defined in DT. > > > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > --- > > > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > > > .../devicetree/bindings/arm/system-suspend.txt | 93 > > > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/arm/system-suspend.txt > > > > > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > > > b/Documentation/devicetree/bindings/arm/psci.txt index > > > 5aa40ede0e99..bd3977a2a333 100644 --- > > > a/Documentation/devicetree/bindings/arm/psci.txt +++ > > > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ Device > > > tree nodes that require usage of PSCI CPU_SUSPEND function (ie idle > > > Definition: power_state parameter to pass to the PSCI suspend call. > > > > > > +PSCI v0.2 and earlier versions don't have explicit operation for system > > > +suspend. However, one can implement system suspend using CPU_SUSPEND by > > > +ensuring every other core except the one executing the CPU_SUSPEND call > > > +has called into PSCI through a CPU_OFF call. > > > > If users explicitly hot-unplug other cores when system load is low to save > > power, then we want to suspend at some point, how does the firmware know > > this case? > > Sorry for confusion. I mean > > If users explicitly hot-unplug other cores when system load is low to save > power, then at some point cpuidle want to suspend the cluster, how does the > distinguish this case with suspend the system to ram. Through the arm,psci-suspend-param DT property, ie PSCI CPU_SUSPEND power_state parameter. Did you read the patch :) ? > > In my private tree, I extend the PSCI spec to tell firmware we want to > > suspend the system. > > I extend the PSCI suspend function to help firmware distinguish suspend system > to ram and other suspend cores/clusters. See above. Thanks, Lorenzo ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-21 13:56 ` Lorenzo Pieralisi @ 2015-01-22 4:33 ` Jisheng Zhang 2015-01-22 6:29 ` Jisheng Zhang 0 siblings, 1 reply; 18+ messages in thread From: Jisheng Zhang @ 2015-01-22 4:33 UTC (permalink / raw) To: linux-arm-kernel Dear Lorenzo, On Wed, 21 Jan 2015 05:56:11 -0800 Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Wed, Jan 21, 2015 at 01:35:07PM +0000, Jisheng Zhang wrote: > > Dear Sudeep, > > > > On Wed, 21 Jan 2015 05:21:39 -0800 > > Jisheng Zhang <jszhang@marvell.com> wrote: > > > > > Dear Sudeep, > > > > > > On Wed, 21 Jan 2015 03:35:54 -0800 > > > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > > ARM based platforms implement unique ways to enter system suspend > > > > (i.e. Suspend to RAM). The mechanism and the parameters defining the > > > > system state vary on a per-platform basis forcing the OS to handle it > > > > in very platform specific way. > > > > > > > > Since ARM 32-bit systems had machine specific code, no attempts to > > > > standardize are being made as it provides easy way to implement > > > > suspend operations in a platform specific manner. However, this > > > > approach not only makes maintainance more difficult as the number of > > > > platforms supported increases but also not feasible for ARM64. > > > > > > > > This DT binding aims at standardizing the system suspend for ARM > > > > platforms. ARM64 platforms mandates entry-method property in DT for > > > > this system suspend node. > > > > > > > > On system implementing PSCI as an enable-method to enter system > > > > suspend, the PSCI CPU suspend method is used on versions upto v0.2 > > > > and requires the power_state parameter to be passed to the PSCI CPU > > > > suspend function. > > > > > > > > This parameter is platform specific, therefore must be provided by > > > > firmware to the OS in order to enable proper call sequence. > > > > > > > > This ARM system suspend DT bindings rely on a property > > > > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > > > > how the PSCI CPU suspend power_state parameter should be defined in > > > > DT. > > > > > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > > --- > > > > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > > > > .../devicetree/bindings/arm/system-suspend.txt | 93 > > > > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/arm/system-suspend.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > > > > b/Documentation/devicetree/bindings/arm/psci.txt index > > > > 5aa40ede0e99..bd3977a2a333 100644 --- > > > > a/Documentation/devicetree/bindings/arm/psci.txt +++ > > > > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ > > > > Device tree nodes that require usage of PSCI CPU_SUSPEND function (ie > > > > idle Definition: power_state parameter to pass to the PSCI suspend > > > > call. > > > > +PSCI v0.2 and earlier versions don't have explicit operation for > > > > system +suspend. However, one can implement system suspend using > > > > CPU_SUSPEND by +ensuring every other core except the one executing > > > > the CPU_SUSPEND call +has called into PSCI through a CPU_OFF call. > > > > > > If users explicitly hot-unplug other cores when system load is low to > > > save power, then we want to suspend at some point, how does the > > > firmware know this case? > > > > Sorry for confusion. I mean > > > > If users explicitly hot-unplug other cores when system load is low to save > > power, then at some point cpuidle want to suspend the cluster, how does > > the distinguish this case with suspend the system to ram. > > Through the arm,psci-suspend-param DT property, ie PSCI CPU_SUSPEND > power_state parameter. > > Did you read the patch :) ? Yep, I do read the patch ;) To be honest, I implemented the s2ram similar as the patch does. But according to PSCI v0.2, "arm,psci-suspend-param = <0x1010000>" means suspend the cluster. I'm not sure I understand it correctly, "can implement system suspend using CPU_SUSPEND by ensuring every other core except the one executing the CPU_SUSPEND call has called into PSCI through a CPU_OFF call" intend to ask firmware to suspend the system if other cores has called into PSCI through a CPU_OFF or suspend the cpu cluster if other cores are not CPU_OFF. I extend the PSCI CPU_SUSPEND function's to use power_state bit[26] to tell firmware whether suspend to ram or not. Could you please correct me if I misunderstand something? Thank you very much, Jisheng > > > > In my private tree, I extend the PSCI spec to tell firmware we want to > > > suspend the system. > > > > I extend the PSCI suspend function to help firmware distinguish suspend > > system to ram and other suspend cores/clusters. > > See above. > > Thanks, > Lorenzo > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-22 4:33 ` Jisheng Zhang @ 2015-01-22 6:29 ` Jisheng Zhang 2015-01-22 11:59 ` Lorenzo Pieralisi 0 siblings, 1 reply; 18+ messages in thread From: Jisheng Zhang @ 2015-01-22 6:29 UTC (permalink / raw) To: linux-arm-kernel Dear Lorenzo and Sudeep, On Wed, 21 Jan 2015 20:33:14 -0800 Jisheng Zhang <jszhang@marvell.com> wrote: > Dear Lorenzo, > > On Wed, 21 Jan 2015 05:56:11 -0800 > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > > On Wed, Jan 21, 2015 at 01:35:07PM +0000, Jisheng Zhang wrote: > > > Dear Sudeep, > > > > > > On Wed, 21 Jan 2015 05:21:39 -0800 > > > Jisheng Zhang <jszhang@marvell.com> wrote: > > > > > > > Dear Sudeep, > > > > > > > > On Wed, 21 Jan 2015 03:35:54 -0800 > > > > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > > > > ARM based platforms implement unique ways to enter system suspend > > > > > (i.e. Suspend to RAM). The mechanism and the parameters defining the > > > > > system state vary on a per-platform basis forcing the OS to handle > > > > > it in very platform specific way. > > > > > > > > > > Since ARM 32-bit systems had machine specific code, no attempts to > > > > > standardize are being made as it provides easy way to implement > > > > > suspend operations in a platform specific manner. However, this > > > > > approach not only makes maintainance more difficult as the number of > > > > > platforms supported increases but also not feasible for ARM64. > > > > > > > > > > This DT binding aims at standardizing the system suspend for ARM > > > > > platforms. ARM64 platforms mandates entry-method property in DT for > > > > > this system suspend node. > > > > > > > > > > On system implementing PSCI as an enable-method to enter system > > > > > suspend, the PSCI CPU suspend method is used on versions upto v0.2 > > > > > and requires the power_state parameter to be passed to the PSCI CPU > > > > > suspend function. > > > > > > > > > > This parameter is platform specific, therefore must be provided by > > > > > firmware to the OS in order to enable proper call sequence. > > > > > > > > > > This ARM system suspend DT bindings rely on a property > > > > > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > > > > > how the PSCI CPU suspend power_state parameter should be defined in > > > > > DT. > > > > > > > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > > > --- > > > > > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > > > > > .../devicetree/bindings/arm/system-suspend.txt | 93 > > > > > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > > > > > create mode 100644 > > > > > Documentation/devicetree/bindings/arm/system-suspend.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > > > > > b/Documentation/devicetree/bindings/arm/psci.txt index > > > > > 5aa40ede0e99..bd3977a2a333 100644 --- > > > > > a/Documentation/devicetree/bindings/arm/psci.txt +++ > > > > > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ > > > > > Device tree nodes that require usage of PSCI CPU_SUSPEND function > > > > > (ie idle Definition: power_state parameter to pass to the PSCI > > > > > suspend call. > > > > > +PSCI v0.2 and earlier versions don't have explicit operation for > > > > > system +suspend. However, one can implement system suspend using > > > > > CPU_SUSPEND by +ensuring every other core except the one executing > > > > > the CPU_SUSPEND call +has called into PSCI through a CPU_OFF call. > > > > > > > > If users explicitly hot-unplug other cores when system load is low to > > > > save power, then we want to suspend at some point, how does the > > > > firmware know this case? > > > > > > Sorry for confusion. I mean > > > > > > If users explicitly hot-unplug other cores when system load is low to > > > save power, then at some point cpuidle want to suspend the cluster, how > > > does the distinguish this case with suspend the system to ram. > > > > Through the arm,psci-suspend-param DT property, ie PSCI CPU_SUSPEND > > power_state parameter. > > > > Did you read the patch :) ? > > Yep, I do read the patch ;) To be honest, I implemented the s2ram similar as > the patch does. But according to PSCI v0.2, "arm,psci-suspend-param = > <0x1010000>" means suspend the cluster. I'm not sure I understand it > correctly, "can implement system suspend using CPU_SUSPEND by ensuring > every other core except the one executing the CPU_SUSPEND call has called > into PSCI through a CPU_OFF call" intend to ask firmware to > > suspend the system if other cores has called into PSCI through a CPU_OFF > > or > > suspend the cpu cluster if other cores are not CPU_OFF. > > > I extend the PSCI CPU_SUSPEND function's to use power_state bit[26] to tell > firmware whether suspend to ram or not. > I read the PSCI spec again, power_state bit[0:15] is "platform specific ID", Is one of these bits used for suspend system? Thanks, Jisheng ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-22 6:29 ` Jisheng Zhang @ 2015-01-22 11:59 ` Lorenzo Pieralisi 2015-01-22 12:09 ` Jisheng Zhang 0 siblings, 1 reply; 18+ messages in thread From: Lorenzo Pieralisi @ 2015-01-22 11:59 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jan 22, 2015 at 06:29:49AM +0000, Jisheng Zhang wrote: > Dear Lorenzo and Sudeep, > > On Wed, 21 Jan 2015 20:33:14 -0800 > Jisheng Zhang <jszhang@marvell.com> wrote: > > > Dear Lorenzo, > > > > On Wed, 21 Jan 2015 05:56:11 -0800 > > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > > > > On Wed, Jan 21, 2015 at 01:35:07PM +0000, Jisheng Zhang wrote: > > > > Dear Sudeep, > > > > > > > > On Wed, 21 Jan 2015 05:21:39 -0800 > > > > Jisheng Zhang <jszhang@marvell.com> wrote: > > > > > > > > > Dear Sudeep, > > > > > > > > > > On Wed, 21 Jan 2015 03:35:54 -0800 > > > > > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > > > > > > ARM based platforms implement unique ways to enter system suspend > > > > > > (i.e. Suspend to RAM). The mechanism and the parameters defining the > > > > > > system state vary on a per-platform basis forcing the OS to handle > > > > > > it in very platform specific way. > > > > > > > > > > > > Since ARM 32-bit systems had machine specific code, no attempts to > > > > > > standardize are being made as it provides easy way to implement > > > > > > suspend operations in a platform specific manner. However, this > > > > > > approach not only makes maintainance more difficult as the number of > > > > > > platforms supported increases but also not feasible for ARM64. > > > > > > > > > > > > This DT binding aims at standardizing the system suspend for ARM > > > > > > platforms. ARM64 platforms mandates entry-method property in DT for > > > > > > this system suspend node. > > > > > > > > > > > > On system implementing PSCI as an enable-method to enter system > > > > > > suspend, the PSCI CPU suspend method is used on versions upto v0.2 > > > > > > and requires the power_state parameter to be passed to the PSCI CPU > > > > > > suspend function. > > > > > > > > > > > > This parameter is platform specific, therefore must be provided by > > > > > > firmware to the OS in order to enable proper call sequence. > > > > > > > > > > > > This ARM system suspend DT bindings rely on a property > > > > > > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > > > > > > how the PSCI CPU suspend power_state parameter should be defined in > > > > > > DT. > > > > > > > > > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > > > > --- > > > > > > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > > > > > > .../devicetree/bindings/arm/system-suspend.txt | 93 > > > > > > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > > > > > > create mode 100644 > > > > > > Documentation/devicetree/bindings/arm/system-suspend.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > > > > > > b/Documentation/devicetree/bindings/arm/psci.txt index > > > > > > 5aa40ede0e99..bd3977a2a333 100644 --- > > > > > > a/Documentation/devicetree/bindings/arm/psci.txt +++ > > > > > > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 +61,14 @@ > > > > > > Device tree nodes that require usage of PSCI CPU_SUSPEND function > > > > > > (ie idle Definition: power_state parameter to pass to the PSCI > > > > > > suspend call. > > > > > > +PSCI v0.2 and earlier versions don't have explicit operation for > > > > > > system +suspend. However, one can implement system suspend using > > > > > > CPU_SUSPEND by +ensuring every other core except the one executing > > > > > > the CPU_SUSPEND call +has called into PSCI through a CPU_OFF call. > > > > > > > > > > If users explicitly hot-unplug other cores when system load is low to > > > > > save power, then we want to suspend at some point, how does the > > > > > firmware know this case? > > > > > > > > Sorry for confusion. I mean > > > > > > > > If users explicitly hot-unplug other cores when system load is low to > > > > save power, then at some point cpuidle want to suspend the cluster, how > > > > does the distinguish this case with suspend the system to ram. > > > > > > Through the arm,psci-suspend-param DT property, ie PSCI CPU_SUSPEND > > > power_state parameter. > > > > > > Did you read the patch :) ? > > > > Yep, I do read the patch ;) To be honest, I implemented the s2ram similar as > > the patch does. But according to PSCI v0.2, "arm,psci-suspend-param = > > <0x1010000>" means suspend the cluster. I'm not sure I understand it > > correctly, "can implement system suspend using CPU_SUSPEND by ensuring > > every other core except the one executing the CPU_SUSPEND call has called > > into PSCI through a CPU_OFF call" intend to ask firmware to > > > > suspend the system if other cores has called into PSCI through a CPU_OFF > > > > or > > > > suspend the cpu cluster if other cores are not CPU_OFF. > > > > > > I extend the PSCI CPU_SUSPEND function's to use power_state bit[26] to tell > > firmware whether suspend to ram or not. > > And that's what the arm,psci-suspend-param stands for in the system-state node. Since system-suspend corresponds supposedly to the highest level of affinity in the system, I would rather say power_state = 0x3010000 can be used for system suspend (affinity bits[25:24] = 0x3), but we did not want to force it, probably that's what we should do. Yes, there is also a platform specific component in power_state param and you can use that too, we wanted to leave flexibility to platforms. PSCI v1.0 will introduce a different separate call for system suspend, this patch copes with "legacy" versions, as the patch logs describe. I agree that the value 0x1010000 was a bad choice for the example, it is confusing, but it does not mean you _have_ to use that value, is it clear ? > I read the PSCI spec again, power_state bit[0:15] is "platform specific ID", > Is one of these bits used for suspend system? It is platform specific, you define that :) ! That's the reason why firmware has to tell the OS what parameter triggers the system-state, it is platform specific, and we provide a binding to define it and provide the OS with the correct value to use. Lorenzo ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-22 11:59 ` Lorenzo Pieralisi @ 2015-01-22 12:09 ` Jisheng Zhang 0 siblings, 0 replies; 18+ messages in thread From: Jisheng Zhang @ 2015-01-22 12:09 UTC (permalink / raw) To: linux-arm-kernel Dear Lorenzo, On Thu, 22 Jan 2015 03:59:06 -0800 Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Thu, Jan 22, 2015 at 06:29:49AM +0000, Jisheng Zhang wrote: > > Dear Lorenzo and Sudeep, > > > > On Wed, 21 Jan 2015 20:33:14 -0800 > > Jisheng Zhang <jszhang@marvell.com> wrote: > > > > > Dear Lorenzo, > > > > > > On Wed, 21 Jan 2015 05:56:11 -0800 > > > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > > > > > > On Wed, Jan 21, 2015 at 01:35:07PM +0000, Jisheng Zhang wrote: > > > > > Dear Sudeep, > > > > > > > > > > On Wed, 21 Jan 2015 05:21:39 -0800 > > > > > Jisheng Zhang <jszhang@marvell.com> wrote: > > > > > > > > > > > Dear Sudeep, > > > > > > > > > > > > On Wed, 21 Jan 2015 03:35:54 -0800 > > > > > > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > > > > > > > > ARM based platforms implement unique ways to enter system > > > > > > > suspend (i.e. Suspend to RAM). The mechanism and the parameters > > > > > > > defining the system state vary on a per-platform basis forcing > > > > > > > the OS to handle it in very platform specific way. > > > > > > > > > > > > > > Since ARM 32-bit systems had machine specific code, no attempts > > > > > > > to standardize are being made as it provides easy way to > > > > > > > implement suspend operations in a platform specific manner. > > > > > > > However, this approach not only makes maintainance more > > > > > > > difficult as the number of platforms supported increases but > > > > > > > also not feasible for ARM64. > > > > > > > > > > > > > > This DT binding aims at standardizing the system suspend for ARM > > > > > > > platforms. ARM64 platforms mandates entry-method property in DT > > > > > > > for this system suspend node. > > > > > > > > > > > > > > On system implementing PSCI as an enable-method to enter system > > > > > > > suspend, the PSCI CPU suspend method is used on versions upto > > > > > > > v0.2 and requires the power_state parameter to be passed to the > > > > > > > PSCI CPU suspend function. > > > > > > > > > > > > > > This parameter is platform specific, therefore must be provided > > > > > > > by firmware to the OS in order to enable proper call sequence. > > > > > > > > > > > > > > This ARM system suspend DT bindings rely on a property > > > > > > > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that > > > > > > > describes how the PSCI CPU suspend power_state parameter should > > > > > > > be defined in DT. > > > > > > > > > > > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > > > > > --- > > > > > > > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > > > > > > > .../devicetree/bindings/arm/system-suspend.txt | 93 > > > > > > > ++++++++++++++++++++++ 2 files changed, 104 insertions(+) > > > > > > > create mode 100644 > > > > > > > Documentation/devicetree/bindings/arm/system-suspend.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt > > > > > > > b/Documentation/devicetree/bindings/arm/psci.txt index > > > > > > > 5aa40ede0e99..bd3977a2a333 100644 --- > > > > > > > a/Documentation/devicetree/bindings/arm/psci.txt +++ > > > > > > > b/Documentation/devicetree/bindings/arm/psci.txt @@ -61,6 > > > > > > > +61,14 @@ Device tree nodes that require usage of PSCI > > > > > > > CPU_SUSPEND function (ie idle Definition: power_state parameter > > > > > > > to pass to the PSCI suspend call. > > > > > > > +PSCI v0.2 and earlier versions don't have explicit operation > > > > > > > for system +suspend. However, one can implement system suspend > > > > > > > using CPU_SUSPEND by +ensuring every other core except the one > > > > > > > executing the CPU_SUSPEND call +has called into PSCI through a > > > > > > > CPU_OFF call. > > > > > > > > > > > > If users explicitly hot-unplug other cores when system load is > > > > > > low to save power, then we want to suspend at some point, how > > > > > > does the firmware know this case? > > > > > > > > > > Sorry for confusion. I mean > > > > > > > > > > If users explicitly hot-unplug other cores when system load is low > > > > > to save power, then at some point cpuidle want to suspend the > > > > > cluster, how does the distinguish this case with suspend the system > > > > > to ram. > > > > > > > > Through the arm,psci-suspend-param DT property, ie PSCI CPU_SUSPEND > > > > power_state parameter. > > > > > > > > Did you read the patch :) ? > > > > > > Yep, I do read the patch ;) To be honest, I implemented the s2ram > > > similar as the patch does. But according to PSCI v0.2, > > > "arm,psci-suspend-param = <0x1010000>" means suspend the cluster. I'm > > > not sure I understand it correctly, "can implement system suspend using > > > CPU_SUSPEND by ensuring every other core except the one executing the > > > CPU_SUSPEND call has called into PSCI through a CPU_OFF call" intend to > > > ask firmware to > > > > > > suspend the system if other cores has called into PSCI through a CPU_OFF > > > > > > or > > > > > > suspend the cpu cluster if other cores are not CPU_OFF. > > > > > > > > > I extend the PSCI CPU_SUSPEND function's to use power_state bit[26] to > > > tell firmware whether suspend to ram or not. > > > > > And that's what the arm,psci-suspend-param stands for in the > system-state node. > > Since system-suspend corresponds supposedly to the highest level of > affinity in the system, I would rather say power_state = 0x3010000 > can be used for system suspend (affinity bits[25:24] = 0x3), but we did > not want to force it, probably that's what we should do. > > Yes, there is also a platform specific component in power_state > param and you can use that too, we wanted to leave flexibility > to platforms. > > PSCI v1.0 will introduce a different separate call for system > suspend, this patch copes with "legacy" versions, as the patch > logs describe. > > I agree that the value 0x1010000 was a bad choice for the example, it > is confusing, but it does not mean you _have_ to use that value, is it > clear ? > > > I read the PSCI spec again, power_state bit[0:15] is "platform specific > > ID", Is one of these bits used for suspend system? > > It is platform specific, you define that :) ! That's the reason why > firmware has to tell the OS what parameter triggers the system-state, > it is platform specific, and we provide a binding to define it and provide > the OS with the correct value to use. > > Lorenzo Thank you for detailed explanations. Now I got your and the patches' points. I were just confused by the 0x1010000. I'll reuse this patch for arm64 suspend system. Thanks, Jisheng ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-01-21 11:35 ` [PATCH RFC 1/2] Documentation: arm: define DT " Sudeep Holla 2015-01-21 13:21 ` Jisheng Zhang @ 2015-02-04 16:10 ` Mark Rutland 2015-02-05 13:28 ` Sudeep Holla 1 sibling, 1 reply; 18+ messages in thread From: Mark Rutland @ 2015-02-04 16:10 UTC (permalink / raw) To: linux-arm-kernel Hi Sudeep, On Wed, Jan 21, 2015 at 11:35:54AM +0000, Sudeep Holla wrote: > ARM based platforms implement unique ways to enter system suspend > (i.e. Suspend to RAM). The mechanism and the parameters defining the > system state vary on a per-platform basis forcing the OS to handle it > in very platform specific way. > > Since ARM 32-bit systems had machine specific code, no attempts to > standardize are being made as it provides easy way to implement suspend > operations in a platform specific manner. However, this approach not > only makes maintainance more difficult as the number of platforms > supported increases but also not feasible for ARM64. > > This DT binding aims at standardizing the system suspend for ARM > platforms. ARM64 platforms mandates entry-method property in DT for > this system suspend node. > > On system implementing PSCI as an enable-method to enter system suspend, > the PSCI CPU suspend method is used on versions upto v0.2 and requires > the power_state parameter to be passed to the PSCI CPU suspend function. > > This parameter is platform specific, therefore must be provided by > firmware to the OS in order to enable proper call sequence. > > This ARM system suspend DT bindings rely on a property > (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > how the PSCI CPU suspend power_state parameter should be defined in DT. A short while ago (after this posting), the PSCI 1.0 spec [1] was released, featuring the new (optional) SYSTEM_SUSPEND mechanism intended for suspend to RAM. This has a standard ID, and its presence can be detected via the new standard PSCI_FEATURES call. The fundamental mechanism is identical. We would hot unplug all but one CPU, and from this remaining CPU we would make a SYSTEM_SUSPEND call. The major differences are that SYSTEM_SUSPEND can be detected via PSCI_FEATURES, and doesn't need a state parameter. Given that the only mandatory addition in PSCI 1.0 over PSCI 0.2 is the PSCI_FEATURES call (used to detect the presence of SYSTEM_SUSPEND), I do not believe that implementing this should be a signficant overhead compared to implementing the CPU_SUSPEND based approach with PSCI 0.2. So I'd very much prefer that we require a minimal PSCI 1.0 with SYSTEM_SUSPEND rather than extending CPU_SUSPEND in this manner. Is anyone attempting to implement suspend to RAM with PSCI 0.1? Thanks, Mark. [1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022c/index.html > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > Documentation/devicetree/bindings/arm/psci.txt | 11 +++ > .../devicetree/bindings/arm/system-suspend.txt | 93 ++++++++++++++++++++++ > 2 files changed, 104 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/system-suspend.txt > > diff --git a/Documentation/devicetree/bindings/arm/psci.txt b/Documentation/devicetree/bindings/arm/psci.txt > index 5aa40ede0e99..bd3977a2a333 100644 > --- a/Documentation/devicetree/bindings/arm/psci.txt > +++ b/Documentation/devicetree/bindings/arm/psci.txt > @@ -61,6 +61,14 @@ Device tree nodes that require usage of PSCI CPU_SUSPEND function (ie idle > Definition: power_state parameter to pass to the PSCI > suspend call. > > +PSCI v0.2 and earlier versions don't have explicit operation for system > +suspend. However, one can implement system suspend using CPU_SUSPEND by > +ensuring every other core except the one executing the CPU_SUSPEND call > +has called into PSCI through a CPU_OFF call. > + > +In such cases, device tree nodes representing system suspend as per the > +bindings in [2] must specify the above "arm,psci-suspend-param" property. > + > Example: > > Case 1: PSCI v0.1 only. > @@ -100,3 +108,6 @@ Case 3: PSCI v0.2 and PSCI v0.1. > > [1] Kernel documentation - ARM idle states bindings > Documentation/devicetree/bindings/arm/idle-states.txt > + > +[2] Kernel documentation - ARM system suspend bindings > + Documentation/devicetree/bindings/arm/system-suspend.txt > diff --git a/Documentation/devicetree/bindings/arm/system-suspend.txt b/Documentation/devicetree/bindings/arm/system-suspend.txt > new file mode 100644 > index 000000000000..15cac4c7fe92 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/system-suspend.txt > @@ -0,0 +1,93 @@ > +========================================== > +ARM system suspend binding description > +========================================== > + > +========================================== > +1 - Introduction > +========================================== > + > +System Suspend(commonly known as Suspend to RAM) is a method to remove > +power from most parts of the machine aside from the RAM, which is required > +to restore the machine's state. Because of the large power savings, it is > +widely used in mobile systems like laptops, tablets and smartphones. > + > +Usually most mobile systems enter system suspend state aggressively when > +they are idle even for short time(in seconds) while others systems like > +laptops automatically enter this mode when they running on batteries and > +the lid is closed (and/or the user is inactive for some time(in minutes)). > + > +Most of the devices in the system are deactivated. Non-volatile memory > +(like disk drives, flash, memory card), graphics chips and even the CPU > +are usually deactivated. Only the RAM is powered to keep its contents. On > +resume, only those individual devices/CPUs need to be reinitialized and > +work continues relatively fast. > + > +It is highly hardware specific especially on ARM platforms. Hence we need > +device tree binding definition for ARM system suspend state which is the > +subject of this document in order to provide generic solution. > + > +=========================================== > +2 - system suspend node > +=========================================== > + > +The system suspend node represents the description of the mechanism to > +enter system suspend state and must be defined as follows: > + > + - compatible > + Usage: Required > + Value type: <stringlist> > + Definition: Must be "arm,system-suspend"; > + > + - entry-method > + Value type: <stringlist> > + Usage and definition depend on ARM architecture version. > + # On ARM v8 64-bit this property is required and must > + be one of: > + - "arm,psci" (see bindings in [1]) > + > + - status: > + Usage: Optional > + Value type: <string> > + Definition: A standard device tree property [2] that indicates > + the operational status of system suspend. > + If present, it shall be: > + "okay": to indicate it is operational. > + "disabled": to indicate that it has been disabled > + in firmware so it is not operational. > + By default, it's always enabled if not explicitly > + disabled. > + > + In addition to the properties listed above, it may require additional > + properties specifics to the entry-method defined, please refer to the > + corresponding entry-method bindings documentation for details. > + In the below example using "arm,psci" entry method, > + "arm,psci-suspend-param" is a PSCI specific property. > + > + The system suspend node's parent node must be the 'cpus' node. > + > +=========================================== > +3 - Examples > +=========================================== > + > +Example: > +cpus { > + /* cpu-map, cpu and idle-states nodes */ > + .... > + > + system-suspend { > + compatible = "arm,system-suspend"; > + entry-method = "arm,psci"; > + arm,psci-suspend-param = <0x1010000>; > + }; > + > + .... > +}; > +=========================================== > +4 - References > +=========================================== > + > +[1] ARM Linux Kernel documentation - PSCI bindings > + Documentation/devicetree/bindings/arm/psci.txt > + > +[2] ePAPR standard > + https://www.power.org/documentation/epapr-version-1-1/ > -- > 1.9.1 > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-02-04 16:10 ` Mark Rutland @ 2015-02-05 13:28 ` Sudeep Holla 2015-02-05 13:32 ` Mark Rutland 0 siblings, 1 reply; 18+ messages in thread From: Sudeep Holla @ 2015-02-05 13:28 UTC (permalink / raw) To: linux-arm-kernel On 04/02/15 16:10, Mark Rutland wrote: > Hi Sudeep, > > On Wed, Jan 21, 2015 at 11:35:54AM +0000, Sudeep Holla wrote: >> ARM based platforms implement unique ways to enter system suspend >> (i.e. Suspend to RAM). The mechanism and the parameters defining the >> system state vary on a per-platform basis forcing the OS to handle it >> in very platform specific way. >> >> Since ARM 32-bit systems had machine specific code, no attempts to >> standardize are being made as it provides easy way to implement suspend >> operations in a platform specific manner. However, this approach not >> only makes maintainance more difficult as the number of platforms >> supported increases but also not feasible for ARM64. >> >> This DT binding aims at standardizing the system suspend for ARM >> platforms. ARM64 platforms mandates entry-method property in DT for >> this system suspend node. >> >> On system implementing PSCI as an enable-method to enter system suspend, >> the PSCI CPU suspend method is used on versions upto v0.2 and requires >> the power_state parameter to be passed to the PSCI CPU suspend function. >> >> This parameter is platform specific, therefore must be provided by >> firmware to the OS in order to enable proper call sequence. >> >> This ARM system suspend DT bindings rely on a property >> (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes >> how the PSCI CPU suspend power_state parameter should be defined in DT. > > A short while ago (after this posting), the PSCI 1.0 spec [1] was > released, featuring the new (optional) SYSTEM_SUSPEND mechanism intended > for suspend to RAM. This has a standard ID, and its presence can be > detected via the new standard PSCI_FEATURES call. > > The fundamental mechanism is identical. We would hot unplug all but one > CPU, and from this remaining CPU we would make a SYSTEM_SUSPEND call. > The major differences are that SYSTEM_SUSPEND can be detected via > PSCI_FEATURES, and doesn't need a state parameter. > > Given that the only mandatory addition in PSCI 1.0 over PSCI 0.2 is the > PSCI_FEATURES call (used to detect the presence of SYSTEM_SUSPEND), I do > not believe that implementing this should be a signficant overhead > compared to implementing the CPU_SUSPEND based approach with PSCI 0.2. > > So I'd very much prefer that we require a minimal PSCI 1.0 with > SYSTEM_SUSPEND rather than extending CPU_SUSPEND in this manner. Is > anyone attempting to implement suspend to RAM with PSCI 0.1? > I too prefer have PSCI v1.0 for SYSTEM SUSPEND support, which eliminates the need for this binding. But the question is: are silicon vendors ready to upgrade their firmware to PSCI v1.0 for system system feature especially since it's one of the fundamental feature needed in Android systems. Regards, Sudeep ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-02-05 13:28 ` Sudeep Holla @ 2015-02-05 13:32 ` Mark Rutland 2015-02-05 13:49 ` Sudeep Holla 0 siblings, 1 reply; 18+ messages in thread From: Mark Rutland @ 2015-02-05 13:32 UTC (permalink / raw) To: linux-arm-kernel On Thu, Feb 05, 2015 at 01:28:32PM +0000, Sudeep Holla wrote: > > > On 04/02/15 16:10, Mark Rutland wrote: > > Hi Sudeep, > > > > On Wed, Jan 21, 2015 at 11:35:54AM +0000, Sudeep Holla wrote: > >> ARM based platforms implement unique ways to enter system suspend > >> (i.e. Suspend to RAM). The mechanism and the parameters defining the > >> system state vary on a per-platform basis forcing the OS to handle it > >> in very platform specific way. > >> > >> Since ARM 32-bit systems had machine specific code, no attempts to > >> standardize are being made as it provides easy way to implement suspend > >> operations in a platform specific manner. However, this approach not > >> only makes maintainance more difficult as the number of platforms > >> supported increases but also not feasible for ARM64. > >> > >> This DT binding aims at standardizing the system suspend for ARM > >> platforms. ARM64 platforms mandates entry-method property in DT for > >> this system suspend node. > >> > >> On system implementing PSCI as an enable-method to enter system suspend, > >> the PSCI CPU suspend method is used on versions upto v0.2 and requires > >> the power_state parameter to be passed to the PSCI CPU suspend function. > >> > >> This parameter is platform specific, therefore must be provided by > >> firmware to the OS in order to enable proper call sequence. > >> > >> This ARM system suspend DT bindings rely on a property > >> (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes > >> how the PSCI CPU suspend power_state parameter should be defined in DT. > > > > A short while ago (after this posting), the PSCI 1.0 spec [1] was > > released, featuring the new (optional) SYSTEM_SUSPEND mechanism intended > > for suspend to RAM. This has a standard ID, and its presence can be > > detected via the new standard PSCI_FEATURES call. > > > > The fundamental mechanism is identical. We would hot unplug all but one > > CPU, and from this remaining CPU we would make a SYSTEM_SUSPEND call. > > The major differences are that SYSTEM_SUSPEND can be detected via > > PSCI_FEATURES, and doesn't need a state parameter. > > > > Given that the only mandatory addition in PSCI 1.0 over PSCI 0.2 is the > > PSCI_FEATURES call (used to detect the presence of SYSTEM_SUSPEND), I do > > not believe that implementing this should be a signficant overhead > > compared to implementing the CPU_SUSPEND based approach with PSCI 0.2. > > > > So I'd very much prefer that we require a minimal PSCI 1.0 with > > SYSTEM_SUSPEND rather than extending CPU_SUSPEND in this manner. Is > > anyone attempting to implement suspend to RAM with PSCI 0.1? > > > > I too prefer have PSCI v1.0 for SYSTEM SUSPEND support, which eliminates > the need for this binding. But the question is: are silicon vendors > ready to upgrade their firmware to PSCI v1.0 for system system feature > especially since it's one of the fundamental feature needed in Android > systems. Sure. That's the question I'd like to know the answer to. If they're bringing up PSCIv0.2 now, the delta to PSCIv0.1 is not large. If they already have an implementation baked, then that's a different scenario. Regardless, what constitutes a wakeup device is a fundamental question to answer, along with what in the system (e.g. peripherals) the kernel must save/restore the state of. Thanks, Mark. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 1/2] Documentation: arm: define DT bindings for system suspend 2015-02-05 13:32 ` Mark Rutland @ 2015-02-05 13:49 ` Sudeep Holla 0 siblings, 0 replies; 18+ messages in thread From: Sudeep Holla @ 2015-02-05 13:49 UTC (permalink / raw) To: linux-arm-kernel On 05/02/15 13:32, Mark Rutland wrote: > On Thu, Feb 05, 2015 at 01:28:32PM +0000, Sudeep Holla wrote: >> >> >> On 04/02/15 16:10, Mark Rutland wrote: >>> Hi Sudeep, >>> >>> On Wed, Jan 21, 2015 at 11:35:54AM +0000, Sudeep Holla wrote: >>>> ARM based platforms implement unique ways to enter system suspend >>>> (i.e. Suspend to RAM). The mechanism and the parameters defining the >>>> system state vary on a per-platform basis forcing the OS to handle it >>>> in very platform specific way. >>>> >>>> Since ARM 32-bit systems had machine specific code, no attempts to >>>> standardize are being made as it provides easy way to implement suspend >>>> operations in a platform specific manner. However, this approach not >>>> only makes maintainance more difficult as the number of platforms >>>> supported increases but also not feasible for ARM64. >>>> >>>> This DT binding aims at standardizing the system suspend for ARM >>>> platforms. ARM64 platforms mandates entry-method property in DT for >>>> this system suspend node. >>>> >>>> On system implementing PSCI as an enable-method to enter system suspend, >>>> the PSCI CPU suspend method is used on versions upto v0.2 and requires >>>> the power_state parameter to be passed to the PSCI CPU suspend function. >>>> >>>> This parameter is platform specific, therefore must be provided by >>>> firmware to the OS in order to enable proper call sequence. >>>> >>>> This ARM system suspend DT bindings rely on a property >>>> (i.e. arm,psci-suspend-param) in the PSCI DT bindings that describes >>>> how the PSCI CPU suspend power_state parameter should be defined in DT. >>> >>> A short while ago (after this posting), the PSCI 1.0 spec [1] was >>> released, featuring the new (optional) SYSTEM_SUSPEND mechanism intended >>> for suspend to RAM. This has a standard ID, and its presence can be >>> detected via the new standard PSCI_FEATURES call. >>> >>> The fundamental mechanism is identical. We would hot unplug all but one >>> CPU, and from this remaining CPU we would make a SYSTEM_SUSPEND call. >>> The major differences are that SYSTEM_SUSPEND can be detected via >>> PSCI_FEATURES, and doesn't need a state parameter. >>> >>> Given that the only mandatory addition in PSCI 1.0 over PSCI 0.2 is the >>> PSCI_FEATURES call (used to detect the presence of SYSTEM_SUSPEND), I do >>> not believe that implementing this should be a signficant overhead >>> compared to implementing the CPU_SUSPEND based approach with PSCI 0.2. >>> >>> So I'd very much prefer that we require a minimal PSCI 1.0 with >>> SYSTEM_SUSPEND rather than extending CPU_SUSPEND in this manner. Is >>> anyone attempting to implement suspend to RAM with PSCI 0.1? >>> >> >> I too prefer have PSCI v1.0 for SYSTEM SUSPEND support, which eliminates >> the need for this binding. But the question is: are silicon vendors >> ready to upgrade their firmware to PSCI v1.0 for system system feature >> especially since it's one of the fundamental feature needed in Android >> systems. > > Sure. That's the question I'd like to know the answer to. > Right, I too hope to get response here. > If they're bringing up PSCIv0.2 now, the delta to PSCIv0.1 is not large. > That's true. > If they already have an implementation baked, then that's a different > scenario. > > Regardless, what constitutes a wakeup device is a fundamental question IIUC individual devices can expose if they are wake-up capable. E.g. RTC can be wakeup capable and if it's enabled(sysfs entry exists, I did use it to test on Juno), it's interrupt is kept unmasked in suspend path while other devices keep their interrupts masked(can be both at GIC and source). > to answer, along with what in the system (e.g. peripherals) the kernel > must save/restore the state of. > Ideally individual drivers need to take care of saving and restoring their state. However there will be always exceptions :) Regards, Sudeep ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND 2015-01-21 11:35 [PATCH RFC 0/2] ARM: DT: add bindings for system suspend Sudeep Holla 2015-01-21 11:35 ` [PATCH RFC 1/2] Documentation: arm: define DT " Sudeep Holla @ 2015-01-21 11:35 ` Sudeep Holla 2015-01-22 6:18 ` Leo Yan 1 sibling, 1 reply; 18+ messages in thread From: Sudeep Holla @ 2015-01-21 11:35 UTC (permalink / raw) To: linux-arm-kernel PSCI specifications upto v0.2 and the related kernel back-end implementation lack a method to enter system wide suspend state. This patch implements suspend to RAM support for all ARM64 systems with PSCIv0.2 support using CPU SUSPEND and the new system state DT bindings. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- arch/arm64/kernel/psci.c | 83 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index f1dbca7d5c96..8be464747dd6 100644 --- a/arch/arm64/kernel/psci.c +++ b/arch/arm64/kernel/psci.c @@ -22,6 +22,7 @@ #include <linux/pm.h> #include <linux/delay.h> #include <linux/slab.h> +#include <linux/suspend.h> #include <uapi/linux/psci.h> #include <asm/compiler.h> @@ -304,6 +305,75 @@ static void psci_sys_poweroff(void) invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); } +static int psci_suspend_finisher(unsigned long arg) +{ + return psci_ops.cpu_suspend(*(struct psci_power_state *)arg, + virt_to_phys(cpu_resume)); +} + +#ifdef CONFIG_SUSPEND +static struct psci_power_state psci_system_suspend_state; + +static int psci_system_suspend_enter(suspend_state_t state) +{ + if (state != PM_SUSPEND_MEM) + return 0; + + /* + * TODO remove pack/unpacking of power_state to do away with + * these ugly type conversions + */ + return __cpu_suspend((unsigned long)&psci_system_suspend_state, + psci_suspend_finisher); +} + +static const struct platform_suspend_ops psci_suspend_ops = { + .valid = suspend_valid_only_mem, + .enter = psci_system_suspend_enter, +}; + +static void __init psci_0_2_system_suspend_init(void) +{ + int ret; + u32 psci_power_state; + const char *entry_method; + struct device_node *node; + + if (!psci_ops.cpu_suspend) + return; /* -EOPNOTSUPP */ + + node = of_find_compatible_node(NULL, NULL, "arm,system-suspend"); + if (!node || !of_device_is_available(node)) + return; /* -EOPNOTSUPP */ + + if (of_property_read_string(node, "entry-method", &entry_method)) { + pr_warn(" * %s missing entry-method property\n", node->full_name); + goto exit; + } + + if (strcmp(entry_method, "arm,psci")) + goto exit; /* out of PSCI scope ignore */ + + ret = of_property_read_u32(node, "arm,psci-suspend-param", + &psci_power_state); + if (ret) { + pr_warn(" * %s missing arm,psci-suspend-param property\n", + node->full_name); + goto exit; + } + + pr_debug("psci-power-state for system suspend %#x\n", psci_power_state); + + psci_power_state_unpack(psci_power_state, &psci_system_suspend_state); + + suspend_set_ops(&psci_suspend_ops); +exit: + of_node_put(node); +} +#else +static void __init psci_0_2_system_suspend_init(void) { } +#endif + /* * PSCI Function IDs for v0.2+ are well defined so use * standard values. @@ -361,6 +431,8 @@ static int __init psci_0_2_init(struct device_node *np) pm_power_off = psci_sys_poweroff; + psci_0_2_system_suspend_init(); + out_put_node: of_node_put(np); return err; @@ -509,14 +581,6 @@ static int cpu_psci_cpu_kill(unsigned int cpu) #endif #endif -static int psci_suspend_finisher(unsigned long index) -{ - struct psci_power_state *state = __this_cpu_read(psci_power_state); - - return psci_ops.cpu_suspend(state[index - 1], - virt_to_phys(cpu_resume)); -} - static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index) { int ret; @@ -531,7 +595,8 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index) if (state[index - 1].type == PSCI_POWER_STATE_TYPE_STANDBY) ret = psci_ops.cpu_suspend(state[index - 1], 0); else - ret = __cpu_suspend(index, psci_suspend_finisher); + ret = __cpu_suspend((unsigned long)&state[index - 1], + psci_suspend_finisher); return ret; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND 2015-01-21 11:35 ` [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND Sudeep Holla @ 2015-01-22 6:18 ` Leo Yan 2015-01-22 12:08 ` Lorenzo Pieralisi 0 siblings, 1 reply; 18+ messages in thread From: Leo Yan @ 2015-01-22 6:18 UTC (permalink / raw) To: linux-arm-kernel On Wed, Jan 21, 2015 at 11:35:55AM +0000, Sudeep Holla wrote: > PSCI specifications upto v0.2 and the related kernel back-end > implementation lack a method to enter system wide suspend state. > > This patch implements suspend to RAM support for all ARM64 systems > with PSCIv0.2 support using CPU SUSPEND and the new system state DT > bindings. > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > --- > arch/arm64/kernel/psci.c | 83 ++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 74 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c > index f1dbca7d5c96..8be464747dd6 100644 > --- a/arch/arm64/kernel/psci.c > +++ b/arch/arm64/kernel/psci.c > @@ -22,6 +22,7 @@ > #include <linux/pm.h> > #include <linux/delay.h> > #include <linux/slab.h> > +#include <linux/suspend.h> > #include <uapi/linux/psci.h> > > #include <asm/compiler.h> > @@ -304,6 +305,75 @@ static void psci_sys_poweroff(void) > invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0); > } > > +static int psci_suspend_finisher(unsigned long arg) > +{ > + return psci_ops.cpu_suspend(*(struct psci_power_state *)arg, > + virt_to_phys(cpu_resume)); > +} > + > +#ifdef CONFIG_SUSPEND > +static struct psci_power_state psci_system_suspend_state; > + > +static int psci_system_suspend_enter(suspend_state_t state) > +{ > + if (state != PM_SUSPEND_MEM) > + return 0; > + > + /* > + * TODO remove pack/unpacking of power_state to do away with > + * these ugly type conversions > + */ > + return __cpu_suspend((unsigned long)&psci_system_suspend_state, > + psci_suspend_finisher); > +} > + > +static const struct platform_suspend_ops psci_suspend_ops = { > + .valid = suspend_valid_only_mem, > + .enter = psci_system_suspend_enter, > +}; > + > +static void __init psci_0_2_system_suspend_init(void) > +{ > + int ret; > + u32 psci_power_state; > + const char *entry_method; > + struct device_node *node; > + > + if (!psci_ops.cpu_suspend) > + return; /* -EOPNOTSUPP */ > + > + node = of_find_compatible_node(NULL, NULL, "arm,system-suspend"); > + if (!node || !of_device_is_available(node)) > + return; /* -EOPNOTSUPP */ > + > + if (of_property_read_string(node, "entry-method", &entry_method)) { > + pr_warn(" * %s missing entry-method property\n", node->full_name); > + goto exit; > + } > + > + if (strcmp(entry_method, "arm,psci")) > + goto exit; /* out of PSCI scope ignore */ > + > + ret = of_property_read_u32(node, "arm,psci-suspend-param", > + &psci_power_state); > + if (ret) { > + pr_warn(" * %s missing arm,psci-suspend-param property\n", > + node->full_name); > + goto exit; > + } > + > + pr_debug("psci-power-state for system suspend %#x\n", psci_power_state); > + > + psci_power_state_unpack(psci_power_state, &psci_system_suspend_state); > + How about unify the power states passing for cpu idle and suspend? below is a example for dts which place all power state into psci entry, so that idle-states and system suspend both can reference the power state. psci { compatible = "arm,psci-0.2"; method = "smc"; power_state { CPU_POWER_OFF: cpu_power_off { state = <0x00010000>; }; CLUSTER_POWER_OFF: cluster_power_off { state = <0x01010000>; }; SOC_SUSPEND: soc_suspend { state = <0x01010001>; }; }; }; > + suspend_set_ops(&psci_suspend_ops); > +exit: > + of_node_put(node); > +} > +#else > +static void __init psci_0_2_system_suspend_init(void) { } > +#endif > + > /* > * PSCI Function IDs for v0.2+ are well defined so use > * standard values. > @@ -361,6 +431,8 @@ static int __init psci_0_2_init(struct device_node *np) > > pm_power_off = psci_sys_poweroff; > > + psci_0_2_system_suspend_init(); > + > out_put_node: > of_node_put(np); > return err; > @@ -509,14 +581,6 @@ static int cpu_psci_cpu_kill(unsigned int cpu) > #endif > #endif > > -static int psci_suspend_finisher(unsigned long index) > -{ > - struct psci_power_state *state = __this_cpu_read(psci_power_state); > - > - return psci_ops.cpu_suspend(state[index - 1], > - virt_to_phys(cpu_resume)); > -} > - > static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index) > { > int ret; > @@ -531,7 +595,8 @@ static int __maybe_unused cpu_psci_cpu_suspend(unsigned long index) > if (state[index - 1].type == PSCI_POWER_STATE_TYPE_STANDBY) > ret = psci_ops.cpu_suspend(state[index - 1], 0); > else > - ret = __cpu_suspend(index, psci_suspend_finisher); > + ret = __cpu_suspend((unsigned long)&state[index - 1], > + psci_suspend_finisher); > > return ret; > } > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND 2015-01-22 6:18 ` Leo Yan @ 2015-01-22 12:08 ` Lorenzo Pieralisi 2015-01-22 14:34 ` Leo Yan 0 siblings, 1 reply; 18+ messages in thread From: Lorenzo Pieralisi @ 2015-01-22 12:08 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jan 22, 2015 at 06:18:12AM +0000, Leo Yan wrote: [...] > How about unify the power states passing for cpu idle and suspend? > > below is a example for dts which place all power state into psci > entry, so that idle-states and system suspend both can reference > the power state. > > psci { > compatible = "arm,psci-0.2"; > method = "smc"; > > power_state { > CPU_POWER_OFF: cpu_power_off { > state = <0x00010000>; > }; > > CLUSTER_POWER_OFF: cluster_power_off { > state = <0x01010000>; > }; > > SOC_SUSPEND: soc_suspend { > state = <0x01010001>; > }; > }; > }; I do not see why this would help. We would end up with phandles to the nodes above to get the parameter from idle-states, to me it looks like churn honestly, I do not see where the improvement is. Lorenzo ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND 2015-01-22 12:08 ` Lorenzo Pieralisi @ 2015-01-22 14:34 ` Leo Yan 2015-01-23 10:58 ` Mark Rutland 0 siblings, 1 reply; 18+ messages in thread From: Leo Yan @ 2015-01-22 14:34 UTC (permalink / raw) To: linux-arm-kernel hi Lorenzo, On Thu, Jan 22, 2015 at 12:08:50PM +0000, Lorenzo Pieralisi wrote: > On Thu, Jan 22, 2015 at 06:18:12AM +0000, Leo Yan wrote: > > [...] > > > How about unify the power states passing for cpu idle and suspend? > > > > below is a example for dts which place all power state into psci > > entry, so that idle-states and system suspend both can reference > > the power state. > > > > psci { > > compatible = "arm,psci-0.2"; > > method = "smc"; > > > > power_state { > > CPU_POWER_OFF: cpu_power_off { > > state = <0x00010000>; > > }; > > > > CLUSTER_POWER_OFF: cluster_power_off { > > state = <0x01010000>; > > }; > > > > SOC_SUSPEND: soc_suspend { > > state = <0x01010001>; > > }; > > }; > > }; > > I do not see why this would help. We would end up with phandles to > the nodes above to get the parameter from idle-states, to me it > looks like churn honestly, I do not see where the improvement is. My original thought is these power states actually are mainly used by the arm trusted firmware; in kernel, it only need maintain such power state ids and pass these power state to firmware according to the requirement from cpuidle or suspend flows. For cpuidle and suspend, they only need to know the index which should use and simply pass this index to the function *cpu_suspend(idx)* will be enough. So finally we also can simplize the code to parse these power state. Following upper idea, the dts can be written like this way: psci { compatible = "arm,psci-0.2"; method = "smc"; power_state { CPU_POWER_OFF: cpu_power_off { state = <0x00010000>; }; CLUSTER_POWER_OFF: cluster_power_off { state = <0x01010000>; }; SOC_SUSPEND: soc_suspend { state = <0x01010001>; }; }; }; idle-states { entry-method = "arm,psci"; C1: cpu-power-down { compatible = "arm,idle-state"; arm,psci-state-idx = 0; entry-latency-us = <20>; exit-latency-us = <40>; min-residency-us = <80>; }; MP: cluster-power-down { compatible = "arm,idle-state"; local-timer-stop; arm,psci-state-idx = 1; entry-latency-us = <50>; exit-latency-us = <100>; min-residency-us = <250>; wakeup-latency-us = <130>; }; }; system-suspend { compatible = "arm,system-suspend"; entry-method = "arm,psci"; arm,psci-state-idx = 2; }; Before i have not followed up the discussion tightly, so if i miss something introduce noise for this topic, sorry about that. Thanks, Leo Yan ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND 2015-01-22 14:34 ` Leo Yan @ 2015-01-23 10:58 ` Mark Rutland 0 siblings, 0 replies; 18+ messages in thread From: Mark Rutland @ 2015-01-23 10:58 UTC (permalink / raw) To: linux-arm-kernel On Thu, Jan 22, 2015 at 02:34:43PM +0000, Leo Yan wrote: > hi Lorenzo, > > On Thu, Jan 22, 2015 at 12:08:50PM +0000, Lorenzo Pieralisi wrote: > > On Thu, Jan 22, 2015 at 06:18:12AM +0000, Leo Yan wrote: > > > > [...] > > > > > How about unify the power states passing for cpu idle and suspend? > > > > > > below is a example for dts which place all power state into psci > > > entry, so that idle-states and system suspend both can reference > > > the power state. > > > > > > psci { > > > compatible = "arm,psci-0.2"; > > > method = "smc"; > > > > > > power_state { > > > CPU_POWER_OFF: cpu_power_off { > > > state = <0x00010000>; > > > }; > > > > > > CLUSTER_POWER_OFF: cluster_power_off { > > > state = <0x01010000>; > > > }; > > > > > > SOC_SUSPEND: soc_suspend { > > > state = <0x01010001>; > > > }; > > > }; > > > }; > > > > I do not see why this would help. We would end up with phandles to > > the nodes above to get the parameter from idle-states, to me it > > looks like churn honestly, I do not see where the improvement is. > > My original thought is these power states actually are mainly used by > the arm trusted firmware; in kernel, it only need maintain such power > state ids and pass these power state to firmware according to the > requirement from cpuidle or suspend flows. > > For cpuidle and suspend, they only need to know the index which > should use and simply pass this index to the function *cpu_suspend(idx)* > will be enough. So finally we also can simplize the code to parse > these power state. > > Following upper idea, the dts can be written like this way: > > psci { > compatible = "arm,psci-0.2"; > method = "smc"; > > power_state { > CPU_POWER_OFF: cpu_power_off { > state = <0x00010000>; > }; > > CLUSTER_POWER_OFF: cluster_power_off { > state = <0x01010000>; > }; > > SOC_SUSPEND: soc_suspend { > state = <0x01010001>; > }; > }; > }; These are only glorified containers for single ID values, and all the information which could potentially have been shared (e.g. local-timer-stop) is not. > > idle-states { > entry-method = "arm,psci"; > > C1: cpu-power-down { > compatible = "arm,idle-state"; > arm,psci-state-idx = 0; This implicitly relies on the ordering of nodes above, which is not a very good idea. As Lorenzo mentioned, we would have to use phandles to explicitly refer to nodes in this matter. So all that we've achieved here is replacing the raw state ID with an indirection to the state ID, and haven't gained any uniformity across idle and suspend states anyway. I don't see the point in just indirecting in this manner unless some additional information is shared. Mark. > entry-latency-us = <20>; > exit-latency-us = <40>; > min-residency-us = <80>; > }; > > MP: cluster-power-down { > compatible = "arm,idle-state"; > local-timer-stop; > arm,psci-state-idx = 1; > entry-latency-us = <50>; > exit-latency-us = <100>; > min-residency-us = <250>; > wakeup-latency-us = <130>; > }; > }; > > system-suspend { > compatible = "arm,system-suspend"; > entry-method = "arm,psci"; > arm,psci-state-idx = 2; > }; > > Before i have not followed up the discussion tightly, so if i miss something > introduce noise for this topic, sorry about that. > > Thanks, > Leo Yan > ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-02-05 13:49 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-21 11:35 [PATCH RFC 0/2] ARM: DT: add bindings for system suspend Sudeep Holla 2015-01-21 11:35 ` [PATCH RFC 1/2] Documentation: arm: define DT " Sudeep Holla 2015-01-21 13:21 ` Jisheng Zhang 2015-01-21 13:35 ` Jisheng Zhang 2015-01-21 13:56 ` Lorenzo Pieralisi 2015-01-22 4:33 ` Jisheng Zhang 2015-01-22 6:29 ` Jisheng Zhang 2015-01-22 11:59 ` Lorenzo Pieralisi 2015-01-22 12:09 ` Jisheng Zhang 2015-02-04 16:10 ` Mark Rutland 2015-02-05 13:28 ` Sudeep Holla 2015-02-05 13:32 ` Mark Rutland 2015-02-05 13:49 ` Sudeep Holla 2015-01-21 11:35 ` [PATCH RFC 2/2] ARM64: psci: implement system suspend using PSCI v0.2 CPU SUSPEND Sudeep Holla 2015-01-22 6:18 ` Leo Yan 2015-01-22 12:08 ` Lorenzo Pieralisi 2015-01-22 14:34 ` Leo Yan 2015-01-23 10:58 ` Mark Rutland
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).