cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
Subsystems (2)
PATCH v3 1/1 cpuidle: tegra: Correctly handle result of arm_cpuidle_simple_enter()
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.
---
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(-)
drivers/cpuidle/cpuidle-tegra.c +20 -14
@@ -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,
Signature · 1 line
2.26.0
09.07.2020 20:35, Dmitry Osipenko пишет:
Quoted 28 lines
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 theenter()failure.Like every other
enter()callback, thearm_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 thearm_cpuidle_simple_enter()returns a error code.The
arm_cpuidle_simple_enter()also may return a-ENODEVerror ifCPU_IDLEis disabled in a kernel's config, but all CPUIDLE drivers are disabled ifCPU_IDLEis disabled, including the cpuidle-tegra driver. So we can't ever see the error code fromarm_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 thearm_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 <[email protected]>
Quoted 8 lines
---
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 ifarm_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?
Quoted 31 lines · Dmitry Osipenko
09.07.2020 20:35, Dmitry Osipenko пишет:
Quoted 30 lines
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 theenter()failure.Like every other
enter()callback, thearm_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 thearm_cpuidle_simple_enter()returns a error code.The
arm_cpuidle_simple_enter()also may return a-ENODEVerror ifCPU_IDLEis disabled in a kernel's config, but all CPUIDLE drivers are disabled ifCPU_IDLEis disabled, including the cpuidle-tegra driver. So we can't ever see the error code fromarm_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 thearm_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 <[email protected]>---
Quoted 9 lines
Quoted 6 lines
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 ifarm_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 ...
Cheers
Jon
Signature · 1 line
nvpublic
24.08.2020 19:02, Jon Hunter пишет:
Quoted 40 lines · Dmitry Osipenko
Quoted 38 lines
09.07.2020 20:35, Dmitry Osipenko пишет:
Quoted 36 lines
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 theenter()failure.Like every other
enter()callback, thearm_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 thearm_cpuidle_simple_enter()returns a error code.The
arm_cpuidle_simple_enter()also may return a-ENODEVerror ifCPU_IDLEis disabled in a kernel's config, but all CPUIDLE drivers are disabled ifCPU_IDLEis disabled, including the cpuidle-tegra driver. So we can't ever see the error code fromarm_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 thearm_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 <[email protected]>---
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 ifarm_cpuidle_simple_enter()will ever fail. Again thanks to Jon Hunter!
Quoted 6 lines
Quoted 2 lines
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 <[email protected]>
No problems, thank you! :)