This change adds idle-state-disabled property using which certain or all
idle-states can be kept disabled during boot-up. Once boot-up is completed
same can be enabled using below command.
echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
Cc: devicetree@vger.kernel.org
Signed-off-by: Tushar Nimkar <quic_tnimkar@quicinc.com>
---
Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
index b8cc826c9501..f999bc666bbd 100644
--- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
+++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
@@ -358,6 +358,13 @@ patternProperties:
systems entry-latency-us + exit-latency-us will exceed
wakeup-latency-us by this duration.
+ idle-state-disabled:
+ description: |
+ If present the idle state stays disabled. It can be enabled back from
+ shell using below command.
+ echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
+ type: boolean
+
idle-state-name:
$ref: /schemas/types.yaml#/definitions/string
description:
@@ -548,6 +555,7 @@ examples:
CPU_SLEEP_0_0: cpu-sleep-0-0 {
compatible = "arm,idle-state";
local-timer-stop;
+ idle-state-disabled;
arm,psci-suspend-param = <0x0010000>;
entry-latency-us = <250>;
exit-latency-us = <500>;
--
2.17.1
On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote: > This change adds idle-state-disabled property using which certain or all > idle-states can be kept disabled during boot-up. Once boot-up is completed > same can be enabled using below command. > > echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable > > Cc: devicetree@vger.kernel.org > Signed-off-by: Tushar Nimkar <quic_tnimkar@quicinc.com> > --- > Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml > index b8cc826c9501..f999bc666bbd 100644 > --- a/Documentation/devicetree/bindings/cpu/idle-states.yaml > +++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml > @@ -358,6 +358,13 @@ patternProperties: > systems entry-latency-us + exit-latency-us will exceed > wakeup-latency-us by this duration. > > + idle-state-disabled: > + description: | > + If present the idle state stays disabled. It can be enabled back from > + shell using below command. > + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable > + type: boolean > + This is clearly a policy and not a hardware or firmware feature to expose in the device tree. So NACK, why can't you load it modules if you don't want idle states in the boot. It is same as choosing any default governor or performance states, will you add those next ? It is simply policy not a feature/property to be exposed in the device tree. -- Regards, Sudeep
Thanks for review Sundeep, On 6/15/2023 2:26 PM, Sudeep Holla wrote: > On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote: >> + idle-state-disabled: >> + description: | >> + If present the idle state stays disabled. It can be enabled back from >> + shell using below command. >> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable >> + type: boolean >> + > > This is clearly a policy and not a hardware or firmware feature to expose > in the device tree. So NACK, why can't you load it modules if you don't want > idle states in the boot. > Attempt of making cpuidle governors to modular was rejected in past [2] [2] https://lore.kernel.org/lkml/1637830481-21709-1-git-send-email-quic_mkshah@quicinc.com/#t > It is same as choosing any default governor or performance states, will you > add those next ? It is simply policy not a feature/property to be exposed > in the device tree. > > -- > Regards, > Sudeep Thanks, Tushar
On Fri, Jun 16, 2023 at 11:26:18AM +0530, Tushar Nimkar wrote: > > Thanks for review Sundeep, > > On 6/15/2023 2:26 PM, Sudeep Holla wrote: > > On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote: > > > + idle-state-disabled: > > > + description: | > > > + If present the idle state stays disabled. It can be enabled back from > > > + shell using below command. > > > + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable > > > + type: boolean > > > + > > > > This is clearly a policy and not a hardware or firmware feature to expose > > in the device tree. So NACK, why can't you load it modules if you don't want > > idle states in the boot. > > > Attempt of making cpuidle governors to modular was rejected in past [2] > OK try command line approach to disable all states(you can't get partial on/off in that case). I don't think the build config is of any use as we end up enabling it which will affect other platforms. > [2] https://lore.kernel.org/lkml/1637830481-21709-1-git-send-email-quic_mkshah@quicinc.com/#t > > > It is same as choosing any default governor or performance states, will you > > add those next ? It is simply policy not a feature/property to be exposed > > in the device tree. > > The above still holds, so still NACK. It is a policy and not a hardware/firmware property or feature. -- Regards, Sudeep
Thanks again Sudeep, On 6/16/2023 9:09 PM, Sudeep Holla wrote: > On Fri, Jun 16, 2023 at 11:26:18AM +0530, Tushar Nimkar wrote: > > OK try command line approach to disable all states(you can't get partial > on/off in that case). I don't think the build config is of any use as we > end up enabling it which will affect other platforms. > Do you mean cpuidle.off=1 ? It will disable idle states but this will not allow cpuidle_init() and governors register to happen which mean no way to re-enable idle states. Do you mean any other command line approach? > > The above still holds, so still NACK. It is a policy and not a > hardware/firmware property or feature. > Yes, understood! Thanks, Tushar
On 08/06/2023 10:55, Tushar Nimkar wrote: > This change adds idle-state-disabled property using which certain or all Please do not use "This commit/patch", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > idle-states can be kept disabled during boot-up. Once boot-up is completed > same can be enabled using below command. > I don't understand and you did not explain here, why this is useful and why this is needed. > echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable > > Cc: devicetree@vger.kernel.org > Signed-off-by: Tushar Nimkar <quic_tnimkar@quicinc.com> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. > --- > Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml > index b8cc826c9501..f999bc666bbd 100644 > --- a/Documentation/devicetree/bindings/cpu/idle-states.yaml > +++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml > @@ -358,6 +358,13 @@ patternProperties: > systems entry-latency-us + exit-latency-us will exceed > wakeup-latency-us by this duration. > > + idle-state-disabled: > + description: | > + If present the idle state stays disabled. It can be enabled back from > + shell using below command. > + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable This is Linux specific command, so does not fit the bindings. Best regards, Krzysztof
Thanks Krzysztof for reviewing, On 6/9/2023 6:44 PM, Krzysztof Kozlowski wrote: > On 08/06/2023 10:55, Tushar Nimkar wrote: >> This change adds idle-state-disabled property using which certain or all > > Please do not use "This commit/patch", but imperative mood. See longer > explanation here: > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > Sure, will update in next version. >> idle-states can be kept disabled during boot-up. Once boot-up is completed >> same can be enabled using below command. >> > > I don't understand and you did not explain here, why this is useful and > why this is needed. > I will update commit text to why this is useful in new version. >> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable > > >> >> Cc: devicetree@vger.kernel.org >> Signed-off-by: Tushar Nimkar <quic_tnimkar@quicinc.com> > > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > Yes, In new version will take care. >> --- >> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable > > This is Linux specific command, so does not fit the bindings. Will remove in new version. > > Best regards, > Krzysztof > Thanks, Tushar
Hey Tushar,
On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote:
> This change adds idle-state-disabled property using which certain or all
> idle-states can be kept disabled during boot-up. Once boot-up is completed
> same can be enabled using below command.
>
> echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
>
> Cc: devicetree@vger.kernel.org
Firstly, you should CC the dt-bindings maintainers like
get_maintainer.pl would tell you.
Secondly, there are two 1/2 patches in this series.
> Signed-off-by: Tushar Nimkar <quic_tnimkar@quicinc.com>
> ---
> Documentation/devicetree/bindings/cpu/idle-states.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/cpu/idle-states.yaml b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> index b8cc826c9501..f999bc666bbd 100644
> --- a/Documentation/devicetree/bindings/cpu/idle-states.yaml
> +++ b/Documentation/devicetree/bindings/cpu/idle-states.yaml
> @@ -358,6 +358,13 @@ patternProperties:
> systems entry-latency-us + exit-latency-us will exceed
> wakeup-latency-us by this duration.
>
> + idle-state-disabled:
> + description: |
> + If present the idle state stays disabled.
> It can be enabled back from
> + shell using below command.
> + echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable
Thirdly, this is operating system specific behaviour, tied to Linux, and
has no place in a binding.
Cheers,
Conor.
> + type: boolean
> +
> idle-state-name:
> $ref: /schemas/types.yaml#/definitions/string
> description:
> @@ -548,6 +555,7 @@ examples:
> CPU_SLEEP_0_0: cpu-sleep-0-0 {
> compatible = "arm,idle-state";
> local-timer-stop;
> + idle-state-disabled;
> arm,psci-suspend-param = <0x0010000>;
> entry-latency-us = <250>;
> exit-latency-us = <500>;
> --
> 2.17.1
>
Thanks Conor for reviewing. On 6/8/2023 2:49 PM, Conor Dooley wrote: > Hey Tushar, > > On Thu, Jun 08, 2023 at 02:25:42PM +0530, Tushar Nimkar wrote: > > Firstly, you should CC the dt-bindings maintainers like > get_maintainer.pl would tell you. > Secondly, there are two 1/2 patches in this series. > 1. Sure, I will include dt maintainer in next version. 2. Yes, one of the Patch 1/2 sent by mistake. I will remove in next version. > > Thirdly, this is operating system specific behaviour, tied to Linux, and > has no place in a binding. > 3. I will remove [echo N > /sys/devices/system/cpu/cpuX/cpuidle/stateX/disable] command from bindings document. > Cheers, > Conor. > Thanks, Tushar
© 2016 - 2026 Red Hat, Inc.