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.

Reviewed-by Jon Hunter <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---

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.

Reviewed-by Jon Hunter <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---

Changelog:

v4: No code changes. Added r-b from Jon Hunter and re-sending for 5.10.

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 -15

index a12fb141875a..e8956706a291 100644
@@ -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 int tegra114_enter_s2idle(struct cpuidle_device *dev,
--

2.27.0