[2/7] intel_idle: cleanup 'intel_idle_init_cstates_icpu()'

Commit Message

Artem Bityutskiy April 19, 2023, 2:39 p.m. UTC

From: Artem Bityutskiy <[e-mail]>

The 'intel_idle_init_cstates_icpu()' function includes a loop that iterates
over every C-state. Inside the loop, the same C-state data is referenced 2
ways:
 1. as 'cpuidle_state_table[cstate]'
 2. as 'drv->states[drv->state_count]' (but it is a copy of #1, not the same
    object).

Make the code be more consistent and easier to read by using only the 2nd
way after the been done. So the code structure would be as follows.

1. Use 'cpuidle_state_table[cstate]'
2. Copy ''cpuidle_state_table[cstate]' to 'drv->states[drv->state_count]'
3. Use only 'drv->states[drv->state_count]' from this point.

Note, this change introduces a checkpatch.pl warning (too long line), but it
will be addressed in the next patch.

Signed-off-by: Artem Bityutskiy <[e-mail]>
---
 drivers/idle/intel_idle.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Zhang Rui April 20, 2023, 1:28 a.m. UTC | #1

Hi, Artem,

On Wed, 2023-04-19 at 17:39 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <[e-mail]>
> 
> The 'intel_idle_init_cstates_icpu()' function includes a loop that
> iterates
> over every C-state. Inside the loop, the same C-state data is
> referenced 2
> ways:
>  1. as 'cpuidle_state_table[cstate]'
>  2. as 'drv->states[drv->state_count]' (but it is a copy of #1, not
> the same
>     object).
> 
> Make the code be more consistent and easier to read by using only the
> 2nd way after

>  the been done.

is this a typo?

thanks,
rui

Artem Bityutskiy April 20, 2023, 6:21 a.m. UTC | #2

On Thu, 2023-04-20 at 01:28 +0000, Zhang, Rui wrote:
> > Make the code be more consistent and easier to read by using only the
> > 2nd way after
> 
> >  the been done.
> 
> is this a typo?

Yes, some sort of editing leftover, will adjust, thanks.

Artem.

Patch

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 726a361da422..190410fc9ce5 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1894,24 +1894,24 @@  static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv)
 		/* Structure copy. */
 		drv->states[drv->state_count] = cpuidle_state_table[cstate];
 
-		if ((cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_IRQ_ENABLE) || force_irq_on) {
+		if ((drv->states[drv->state_count].flags & CPUIDLE_FLAG_IRQ_ENABLE) || force_irq_on) {
 			pr_info("forced intel_idle_irq for state %d\n", cstate);
 			drv->states[drv->state_count].enter = intel_idle_irq;
 		}
 
 		if (cpu_feature_enabled(X86_FEATURE_KERNEL_IBRS) &&
-		    cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_IBRS) {
-			WARN_ON_ONCE(cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_IRQ_ENABLE);
+		    drv->states[drv->state_count].flags & CPUIDLE_FLAG_IBRS) {
+			WARN_ON_ONCE(drv->states[drv->state_count].flags & CPUIDLE_FLAG_IRQ_ENABLE);
 			drv->states[drv->state_count].enter = intel_idle_ibrs;
 		}
 
-		if (cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_INIT_XSTATE)
+		if (drv->states[drv->state_count].flags & CPUIDLE_FLAG_INIT_XSTATE)
 			drv->states[drv->state_count].enter = intel_idle_xstate;
 
 		if ((disabled_states_mask & BIT(drv->state_count)) ||
 		    ((icpu->use_acpi || force_use_acpi) &&
 		     intel_idle_off_by_default(mwait_hint) &&
-		     !(cpuidle_state_table[cstate].flags & CPUIDLE_FLAG_ALWAYS_ENABLE)))
+		     !(drv->states[drv->state_count].flags & CPUIDLE_FLAG_ALWAYS_ENABLE)))
 			drv->states[drv->state_count].flags |= CPUIDLE_FLAG_OFF;
 
 		if (intel_idle_state_needs_timer_stop(&drv->states[drv->state_count]))