diff mbox series

[2/7] intel_idle: clean up the (new) state_update_enter_method function

Message ID 20230601182801.2622044-3-arjan@linux.intel.com (mailing list archive)
State Superseded, archived
Headers show
Series [1/7] intel_idle: refactor state->enter manipulation into its own function | expand

Commit Message

Arjan van de Ven June 1, 2023, 6:27 p.m. UTC 
From: Arjan van de Ven <arjan.van.de.ven@intel.com>

Now that the logic for state_update_enter_method() is in its own
function, the long if .. else if .. else if .. else if chain
can be simplified by just returning from the function
at the various places. This does not change functionality,
but it makes the logic much simpler to read or modify later.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/idle/intel_idle.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki June 4, 2023, 3:34 p.m. UTC | #1 
On Thu, Jun 1, 2023 at 8:28 PM <arjan@linux.intel.com> wrote:
>
> From: Arjan van de Ven <arjan.van.de.ven@intel.com>
>
> Now that the logic for state_update_enter_method() is in its own
> function, the long if .. else if .. else if .. else if chain
> can be simplified by just returning from the function
> at the various places. This does not change functionality,
> but it makes the logic much simpler to read or modify later.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

This and the [1/7] can be applied without the rest of the series, so
please let me know if you want me to do that.

> ---
>  drivers/idle/intel_idle.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index c351b21c0875..256c2d42e350 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1849,7 +1849,10 @@ static void state_update_enter_method(struct cpuidle_state *state, int cstate)
>                 WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IBRS);
>                 WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
>                 state->enter = intel_idle_xstate;
> -       } else if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
> +               return;
> +       }
> +
> +       if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
>                            state->flags & CPUIDLE_FLAG_IBRS) {
>                 /*
>                  * IBRS mitigation requires that C-states are entered
> @@ -1857,9 +1860,15 @@ static void state_update_enter_method(struct cpuidle_state *state, int cstate)
>                  */
>                 WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
>                 state->enter = intel_idle_ibrs;
> -       } else if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
> +               return;
> +       }
> +
> +       if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
>                 state->enter = intel_idle_irq;
> -       } else if (force_irq_on) {
> +               return;
> +       }
> +
> +       if (force_irq_on) {
>                 pr_info("forced intel_idle_irq for state %d\n", cstate);
>                 state->enter = intel_idle_irq;
>         }
> --
> 2.40.1
>
Van De Ven, Arjan June 4, 2023, 10:35 p.m. UTC | #2 
> 
> This and the [1/7] can be applied without the rest of the series, so
> please let me know if you want me to do that.


yes please do that; it's a useful cleanup either way and makes the next version of the series smaller/simpler
diff mbox series

Patch

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index c351b21c0875..256c2d42e350 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1849,7 +1849,10 @@  static void state_update_enter_method(struct cpuidle_state *state, int cstate)
 		WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IBRS);
 		WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
 		state->enter = intel_idle_xstate;
-	} else if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
+		return;
+	}
+
+	if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
 			   state->flags & CPUIDLE_FLAG_IBRS) {
 		/*
 		 * IBRS mitigation requires that C-states are entered
@@ -1857,9 +1860,15 @@  static void state_update_enter_method(struct cpuidle_state *state, int cstate)
 		 */
 		WARN_ON_ONCE(state->flags & CPUIDLE_FLAG_IRQ_ENABLE);
 		state->enter = intel_idle_ibrs;
-	} else if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
+		return;
+	}
+
+	if (state->flags & CPUIDLE_FLAG_IRQ_ENABLE) {
 		state->enter = intel_idle_irq;
-	} else if (force_irq_on) {
+		return;
+	}
+
+	if (force_irq_on) {
 		pr_info("forced intel_idle_irq for state %d\n", cstate);
 		state->enter = intel_idle_irq;
 	}