* [PATCH v3] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
@ 2020-07-09 17:35 ` Dmitry Osipenko
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2020-07-09 17:35 UTC (permalink / raw
To: Thierry Reding, Jonathan Hunter, Rafael J. Wysocki,
Daniel Lezcano
Cc: linux-pm, linux-tegra
The enter() callback of CPUIDLE drivers returns index of the entered idle
state on success or a negative value on failure. The negative value could
any negative value, i.e. it doesn't necessarily needs to be a error code.
That's because CPUIDLE core only cares about the fact of failure and not
about the reason of the enter() failure.
Like every other enter() callback, the arm_cpuidle_simple_enter() returns
the entered idle-index on success. Unlike some of other drivers, it never
fails. It happened that TEGRA_C1=index=err=0 in the code of cpuidle-tegra
driver, and thus, there is no problem for the cpuidle-tegra driver created
by the typo in the code which assumes that the arm_cpuidle_simple_enter()
returns a error code.
The arm_cpuidle_simple_enter() also may return a -ENODEV error if CPU_IDLE
is disabled in a kernel's config, but all CPUIDLE drivers are disabled if
CPU_IDLE is disabled, including the cpuidle-tegra driver. So we can't ever
see the error code from arm_cpuidle_simple_enter() today.
Of course the code may get some changes in the future and then the
typo may transform into a real bug, so let's correct the typo! The
tegra_cpuidle_state_enter() is now changed to make it return the entered
idle-index on success and negative error code on fail, which puts it on
par with the arm_cpuidle_simple_enter(), making code consistent in regards
to the error handling.
This patch fixes a minor typo in the code, it doesn't fix any bugs.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
Changelog:
v3: The tegra_cpuidle_state_enter() now returns entered idle-index on
success instead of 0. Hence the error message will be shown by the
tegra-cpuidle driver if arm_cpuidle_simple_enter() will ever fail.
Again thanks to Jon Hunter!
v2: Improved commit message by clarifying what values are returned by
arm_cpuidle_simple_enter() and when. Thanks to Jon Hunter for the
suggestion!
drivers/cpuidle/cpuidle-tegra.c | 34 +++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index 150045849d78..30e6816c6737 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -172,7 +172,7 @@ static int tegra_cpuidle_coupled_barrier(struct cpuidle_device *dev)
static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
int index, unsigned int cpu)
{
- int ret;
+ int err;
/*
* CC6 state is the "CPU cluster power-off" state. In order to
@@ -183,9 +183,9 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
* CPU cores, GIC and L2 cache).
*/
if (index == TEGRA_CC6) {
- ret = tegra_cpuidle_coupled_barrier(dev);
- if (ret)
- return ret;
+ err = tegra_cpuidle_coupled_barrier(dev);
+ if (err)
+ return err;
}
local_fiq_disable();
@@ -194,15 +194,15 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
switch (index) {
case TEGRA_C7:
- ret = tegra_cpuidle_c7_enter();
+ err = tegra_cpuidle_c7_enter();
break;
case TEGRA_CC6:
- ret = tegra_cpuidle_cc6_enter(cpu);
+ err = tegra_cpuidle_cc6_enter(cpu);
break;
default:
- ret = -EINVAL;
+ err = -EINVAL;
break;
}
@@ -210,7 +210,7 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
tegra_pm_clear_cpu_in_lp2();
local_fiq_enable();
- return ret;
+ return err ?: index;
}
static int tegra_cpuidle_adjust_state_index(int index, unsigned int cpu)
@@ -236,21 +236,27 @@ static int tegra_cpuidle_enter(struct cpuidle_device *dev,
int index)
{
unsigned int cpu = cpu_logical_map(dev->cpu);
- int err;
+ int ret;
index = tegra_cpuidle_adjust_state_index(index, cpu);
if (dev->states_usage[index].disable)
return -1;
if (index == TEGRA_C1)
- err = arm_cpuidle_simple_enter(dev, drv, index);
+ ret = arm_cpuidle_simple_enter(dev, drv, index);
else
- err = tegra_cpuidle_state_enter(dev, index, cpu);
+ ret = tegra_cpuidle_state_enter(dev, index, cpu);
- if (err && (err != -EINTR || index != TEGRA_CC6))
- pr_err_once("failed to enter state %d err: %d\n", index, err);
+ if (ret < 0) {
+ if (ret != -EINTR || index != TEGRA_CC6)
+ pr_err_once("failed to enter state %d err: %d\n",
+ index, ret);
+ index = -1;
+ } else {
+ index = ret;
+ }
- return err ? -1 : index;
+ return index;
}
static void tegra114_enter_s2idle(struct cpuidle_device *dev,
--
2.26.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
@ 2020-07-09 17:35 ` Dmitry Osipenko
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2020-07-09 17:35 UTC (permalink / raw
To: Thierry Reding, Jonathan Hunter, Rafael J. Wysocki,
Daniel Lezcano
Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
The enter() callback of CPUIDLE drivers returns index of the entered idle
state on success or a negative value on failure. The negative value could
any negative value, i.e. it doesn't necessarily needs to be a error code.
That's because CPUIDLE core only cares about the fact of failure and not
about the reason of the enter() failure.
Like every other enter() callback, the arm_cpuidle_simple_enter() returns
the entered idle-index on success. Unlike some of other drivers, it never
fails. It happened that TEGRA_C1=index=err=0 in the code of cpuidle-tegra
driver, and thus, there is no problem for the cpuidle-tegra driver created
by the typo in the code which assumes that the arm_cpuidle_simple_enter()
returns a error code.
The arm_cpuidle_simple_enter() also may return a -ENODEV error if CPU_IDLE
is disabled in a kernel's config, but all CPUIDLE drivers are disabled if
CPU_IDLE is disabled, including the cpuidle-tegra driver. So we can't ever
see the error code from arm_cpuidle_simple_enter() today.
Of course the code may get some changes in the future and then the
typo may transform into a real bug, so let's correct the typo! The
tegra_cpuidle_state_enter() is now changed to make it return the entered
idle-index on success and negative error code on fail, which puts it on
par with the arm_cpuidle_simple_enter(), making code consistent in regards
to the error handling.
This patch fixes a minor typo in the code, it doesn't fix any bugs.
Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Changelog:
v3: The tegra_cpuidle_state_enter() now returns entered idle-index on
success instead of 0. Hence the error message will be shown by the
tegra-cpuidle driver if arm_cpuidle_simple_enter() will ever fail.
Again thanks to Jon Hunter!
v2: Improved commit message by clarifying what values are returned by
arm_cpuidle_simple_enter() and when. Thanks to Jon Hunter for the
suggestion!
drivers/cpuidle/cpuidle-tegra.c | 34 +++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index 150045849d78..30e6816c6737 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -172,7 +172,7 @@ static int tegra_cpuidle_coupled_barrier(struct cpuidle_device *dev)
static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
int index, unsigned int cpu)
{
- int ret;
+ int err;
/*
* CC6 state is the "CPU cluster power-off" state. In order to
@@ -183,9 +183,9 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
* CPU cores, GIC and L2 cache).
*/
if (index == TEGRA_CC6) {
- ret = tegra_cpuidle_coupled_barrier(dev);
- if (ret)
- return ret;
+ err = tegra_cpuidle_coupled_barrier(dev);
+ if (err)
+ return err;
}
local_fiq_disable();
@@ -194,15 +194,15 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
switch (index) {
case TEGRA_C7:
- ret = tegra_cpuidle_c7_enter();
+ err = tegra_cpuidle_c7_enter();
break;
case TEGRA_CC6:
- ret = tegra_cpuidle_cc6_enter(cpu);
+ err = tegra_cpuidle_cc6_enter(cpu);
break;
default:
- ret = -EINVAL;
+ err = -EINVAL;
break;
}
@@ -210,7 +210,7 @@ static int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
tegra_pm_clear_cpu_in_lp2();
local_fiq_enable();
- return ret;
+ return err ?: index;
}
static int tegra_cpuidle_adjust_state_index(int index, unsigned int cpu)
@@ -236,21 +236,27 @@ static int tegra_cpuidle_enter(struct cpuidle_device *dev,
int index)
{
unsigned int cpu = cpu_logical_map(dev->cpu);
- int err;
+ int ret;
index = tegra_cpuidle_adjust_state_index(index, cpu);
if (dev->states_usage[index].disable)
return -1;
if (index == TEGRA_C1)
- err = arm_cpuidle_simple_enter(dev, drv, index);
+ ret = arm_cpuidle_simple_enter(dev, drv, index);
else
- err = tegra_cpuidle_state_enter(dev, index, cpu);
+ ret = tegra_cpuidle_state_enter(dev, index, cpu);
- if (err && (err != -EINTR || index != TEGRA_CC6))
- pr_err_once("failed to enter state %d err: %d\n", index, err);
+ if (ret < 0) {
+ if (ret != -EINTR || index != TEGRA_CC6)
+ pr_err_once("failed to enter state %d err: %d\n",
+ index, ret);
+ index = -1;
+ } else {
+ index = ret;
+ }
- return err ? -1 : index;
+ return index;
}
static void tegra114_enter_s2idle(struct cpuidle_device *dev,
--
2.26.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
2020-07-09 17:35 ` Dmitry Osipenko
(?)
@ 2020-08-14 17:53 ` Dmitry Osipenko
2020-08-24 16:02 ` Jon Hunter
-1 siblings, 1 reply; 5+ messages in thread
From: Dmitry Osipenko @ 2020-08-14 17:53 UTC (permalink / raw
To: Thierry Reding, Jonathan Hunter, Rafael J. Wysocki,
Daniel Lezcano
Cc: linux-pm, linux-tegra
09.07.2020 20:35, Dmitry Osipenko пишет:
> The enter() callback of CPUIDLE drivers returns index of the entered idle
> state on success or a negative value on failure. The negative value could
> any negative value, i.e. it doesn't necessarily needs to be a error code.
> That's because CPUIDLE core only cares about the fact of failure and not
> about the reason of the enter() failure.
>
> Like every other enter() callback, the arm_cpuidle_simple_enter() returns
> the entered idle-index on success. Unlike some of other drivers, it never
> fails. It happened that TEGRA_C1=index=err=0 in the code of cpuidle-tegra
> driver, and thus, there is no problem for the cpuidle-tegra driver created
> by the typo in the code which assumes that the arm_cpuidle_simple_enter()
> returns a error code.
>
> The arm_cpuidle_simple_enter() also may return a -ENODEV error if CPU_IDLE
> is disabled in a kernel's config, but all CPUIDLE drivers are disabled if
> CPU_IDLE is disabled, including the cpuidle-tegra driver. So we can't ever
> see the error code from arm_cpuidle_simple_enter() today.
>
> Of course the code may get some changes in the future and then the
> typo may transform into a real bug, so let's correct the typo! The
> tegra_cpuidle_state_enter() is now changed to make it return the entered
> idle-index on success and negative error code on fail, which puts it on
> par with the arm_cpuidle_simple_enter(), making code consistent in regards
> to the error handling.
>
> This patch fixes a minor typo in the code, it doesn't fix any bugs.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>
> Changelog:
>
> v3: The tegra_cpuidle_state_enter() now returns entered idle-index on
> success instead of 0. Hence the error message will be shown by the
> tegra-cpuidle driver if arm_cpuidle_simple_enter() will ever fail.
> Again thanks to Jon Hunter!
Hello, Jon! Do you see anything else that could be improved in this patch?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
2020-08-14 17:53 ` Dmitry Osipenko
@ 2020-08-24 16:02 ` Jon Hunter
2020-08-24 16:22 ` Dmitry Osipenko
0 siblings, 1 reply; 5+ messages in thread
From: Jon Hunter @ 2020-08-24 16:02 UTC (permalink / raw
To: Dmitry Osipenko, Thierry Reding, Rafael J. Wysocki,
Daniel Lezcano
Cc: linux-pm, linux-tegra
On 14/08/2020 18:53, Dmitry Osipenko wrote:
> 09.07.2020 20:35, Dmitry Osipenko пишет:
>> The enter() callback of CPUIDLE drivers returns index of the entered idle
>> state on success or a negative value on failure. The negative value could
>> any negative value, i.e. it doesn't necessarily needs to be a error code.
>> That's because CPUIDLE core only cares about the fact of failure and not
>> about the reason of the enter() failure.
>>
>> Like every other enter() callback, the arm_cpuidle_simple_enter() returns
>> the entered idle-index on success. Unlike some of other drivers, it never
>> fails. It happened that TEGRA_C1=index=err=0 in the code of cpuidle-tegra
>> driver, and thus, there is no problem for the cpuidle-tegra driver created
>> by the typo in the code which assumes that the arm_cpuidle_simple_enter()
>> returns a error code.
>>
>> The arm_cpuidle_simple_enter() also may return a -ENODEV error if CPU_IDLE
>> is disabled in a kernel's config, but all CPUIDLE drivers are disabled if
>> CPU_IDLE is disabled, including the cpuidle-tegra driver. So we can't ever
>> see the error code from arm_cpuidle_simple_enter() today.
>>
>> Of course the code may get some changes in the future and then the
>> typo may transform into a real bug, so let's correct the typo! The
>> tegra_cpuidle_state_enter() is now changed to make it return the entered
>> idle-index on success and negative error code on fail, which puts it on
>> par with the arm_cpuidle_simple_enter(), making code consistent in regards
>> to the error handling.
>>
>> This patch fixes a minor typo in the code, it doesn't fix any bugs.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>
>> Changelog:
>>
>> v3: The tegra_cpuidle_state_enter() now returns entered idle-index on
>> success instead of 0. Hence the error message will be shown by the
>> tegra-cpuidle driver if arm_cpuidle_simple_enter() will ever fail.
>> Again thanks to Jon Hunter!
>
> Hello, Jon! Do you see anything else that could be improved in this patch?
>
Sorry for the delay. Looks good to me ...
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
2020-08-24 16:02 ` Jon Hunter
@ 2020-08-24 16:22 ` Dmitry Osipenko
0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Osipenko @ 2020-08-24 16:22 UTC (permalink / raw
To: Jon Hunter, Thierry Reding, Rafael J. Wysocki, Daniel Lezcano
Cc: linux-pm, linux-tegra
24.08.2020 19:02, Jon Hunter пишет:
>
> On 14/08/2020 18:53, Dmitry Osipenko wrote:
>> 09.07.2020 20:35, Dmitry Osipenko пишет:
>>> The enter() callback of CPUIDLE drivers returns index of the entered idle
>>> state on success or a negative value on failure. The negative value could
>>> any negative value, i.e. it doesn't necessarily needs to be a error code.
>>> That's because CPUIDLE core only cares about the fact of failure and not
>>> about the reason of the enter() failure.
>>>
>>> Like every other enter() callback, the arm_cpuidle_simple_enter() returns
>>> the entered idle-index on success. Unlike some of other drivers, it never
>>> fails. It happened that TEGRA_C1=index=err=0 in the code of cpuidle-tegra
>>> driver, and thus, there is no problem for the cpuidle-tegra driver created
>>> by the typo in the code which assumes that the arm_cpuidle_simple_enter()
>>> returns a error code.
>>>
>>> The arm_cpuidle_simple_enter() also may return a -ENODEV error if CPU_IDLE
>>> is disabled in a kernel's config, but all CPUIDLE drivers are disabled if
>>> CPU_IDLE is disabled, including the cpuidle-tegra driver. So we can't ever
>>> see the error code from arm_cpuidle_simple_enter() today.
>>>
>>> Of course the code may get some changes in the future and then the
>>> typo may transform into a real bug, so let's correct the typo! The
>>> tegra_cpuidle_state_enter() is now changed to make it return the entered
>>> idle-index on success and negative error code on fail, which puts it on
>>> par with the arm_cpuidle_simple_enter(), making code consistent in regards
>>> to the error handling.
>>>
>>> This patch fixes a minor typo in the code, it doesn't fix any bugs.
>>>
>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>> ---
>>>
>>> Changelog:
>>>
>>> v3: The tegra_cpuidle_state_enter() now returns entered idle-index on
>>> success instead of 0. Hence the error message will be shown by the
>>> tegra-cpuidle driver if arm_cpuidle_simple_enter() will ever fail.
>>> Again thanks to Jon Hunter!
>>
>> Hello, Jon! Do you see anything else that could be improved in this patch?
>>
>
> Sorry for the delay. Looks good to me ...
>
> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
No problems, thank you! :)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-24 16:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-09 17:35 [PATCH v3] cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter() Dmitry Osipenko
2020-07-09 17:35 ` Dmitry Osipenko
2020-08-14 17:53 ` Dmitry Osipenko
2020-08-24 16:02 ` Jon Hunter
2020-08-24 16:22 ` Dmitry Osipenko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.