* [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
[not found] <20230608085544.16211-1-quic_tnimkar@quicinc.com>
@ 2023-06-08 8:55 ` Tushar Nimkar
2023-06-08 9:19 ` Conor Dooley
` (2 more replies)
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: cpu: " Tushar Nimkar
1 sibling, 3 replies; 11+ messages in thread
From: Tushar Nimkar @ 2023-06-08 8:55 UTC (permalink / raw
To: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson
Cc: linux-pm, linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
Tushar Nimkar, devicetree
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 1/2] dt-bindings: cpu: idle-states: Add idle-state-disabled property
[not found] <20230608085544.16211-1-quic_tnimkar@quicinc.com>
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property Tushar Nimkar
@ 2023-06-08 8:55 ` Tushar Nimkar
2023-06-09 13:15 ` Krzysztof Kozlowski
1 sibling, 1 reply; 11+ messages in thread
From: Tushar Nimkar @ 2023-06-08 8:55 UTC (permalink / raw
To: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson
Cc: linux-pm, linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
Tushar Nimkar, devicetree
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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property Tushar Nimkar
@ 2023-06-08 9:19 ` Conor Dooley
2023-06-09 4:39 ` Tushar Nimkar
2023-06-09 13:14 ` Krzysztof Kozlowski
2023-06-15 8:56 ` Sudeep Holla
2 siblings, 1 reply; 11+ messages in thread
From: Conor Dooley @ 2023-06-08 9:19 UTC (permalink / raw
To: Tushar Nimkar
Cc: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson, linux-pm,
linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah, devicetree
[-- Attachment #1: Type: text/plain, Size: 2085 bytes --]
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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-08 9:19 ` Conor Dooley
@ 2023-06-09 4:39 ` Tushar Nimkar
0 siblings, 0 replies; 11+ messages in thread
From: Tushar Nimkar @ 2023-06-09 4:39 UTC (permalink / raw
To: Conor Dooley
Cc: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson, linux-pm,
linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah, devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property Tushar Nimkar
2023-06-08 9:19 ` Conor Dooley
@ 2023-06-09 13:14 ` Krzysztof Kozlowski
2023-06-14 6:45 ` Tushar Nimkar
2023-06-15 8:56 ` Sudeep Holla
2 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-09 13:14 UTC (permalink / raw
To: Tushar Nimkar, Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson
Cc: linux-pm, linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: cpu: idle-states: Add idle-state-disabled property
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: cpu: " Tushar Nimkar
@ 2023-06-09 13:15 ` Krzysztof Kozlowski
0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2023-06-09 13:15 UTC (permalink / raw
To: Tushar Nimkar, Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson
Cc: linux-pm, linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
devicetree
On 08/06/2023 10:55, 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/disab
This is a duplicated patch. I already commented on other so to make it
clear: NAK for this one :)
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-09 13:14 ` Krzysztof Kozlowski
@ 2023-06-14 6:45 ` Tushar Nimkar
0 siblings, 0 replies; 11+ messages in thread
From: Tushar Nimkar @ 2023-06-14 6:45 UTC (permalink / raw
To: Krzysztof Kozlowski, Rafael J . Wysocki, Daniel Lezcano,
Ulf Hansson
Cc: linux-pm, linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property Tushar Nimkar
2023-06-08 9:19 ` Conor Dooley
2023-06-09 13:14 ` Krzysztof Kozlowski
@ 2023-06-15 8:56 ` Sudeep Holla
2023-06-16 5:56 ` Tushar Nimkar
2 siblings, 1 reply; 11+ messages in thread
From: Sudeep Holla @ 2023-06-15 8:56 UTC (permalink / raw
To: Tushar Nimkar
Cc: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson, linux-pm,
linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
Sudeep Holla, devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-15 8:56 ` Sudeep Holla
@ 2023-06-16 5:56 ` Tushar Nimkar
2023-06-16 15:39 ` Sudeep Holla
0 siblings, 1 reply; 11+ messages in thread
From: Tushar Nimkar @ 2023-06-16 5:56 UTC (permalink / raw
To: Sudeep Holla
Cc: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson, linux-pm,
linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah, devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-16 5:56 ` Tushar Nimkar
@ 2023-06-16 15:39 ` Sudeep Holla
2023-06-21 6:27 ` Tushar Nimkar
0 siblings, 1 reply; 11+ messages in thread
From: Sudeep Holla @ 2023-06-16 15:39 UTC (permalink / raw
To: Tushar Nimkar
Cc: Rafael J . Wysocki, Sudeep Holla, Daniel Lezcano, Ulf Hansson,
linux-pm, linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah,
devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property
2023-06-16 15:39 ` Sudeep Holla
@ 2023-06-21 6:27 ` Tushar Nimkar
0 siblings, 0 replies; 11+ messages in thread
From: Tushar Nimkar @ 2023-06-21 6:27 UTC (permalink / raw
To: Sudeep Holla
Cc: Rafael J . Wysocki, Daniel Lezcano, Ulf Hansson, linux-pm,
linux-arm-msm, linux-kernel, quic_lsrao, quic_mkshah, devicetree
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-06-21 6:27 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230608085544.16211-1-quic_tnimkar@quicinc.com>
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: arm: idle-states: Add idle-state-disabled property Tushar Nimkar
2023-06-08 9:19 ` Conor Dooley
2023-06-09 4:39 ` Tushar Nimkar
2023-06-09 13:14 ` Krzysztof Kozlowski
2023-06-14 6:45 ` Tushar Nimkar
2023-06-15 8:56 ` Sudeep Holla
2023-06-16 5:56 ` Tushar Nimkar
2023-06-16 15:39 ` Sudeep Holla
2023-06-21 6:27 ` Tushar Nimkar
2023-06-08 8:55 ` [PATCH 1/2] dt-bindings: cpu: " Tushar Nimkar
2023-06-09 13:15 ` Krzysztof Kozlowski
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 read-only IMAP folder(s) and NNTP newsgroup(s).