diff mbox series

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

Message ID 20230420064718.1981936-1-dedekind1@gmail.com (mailing list archive)
State Mainlined, archived
Headers show
Series None | expand

Commit Message

Artem Bityutskiy April 20, 2023, 6:47 a.m. UTC 
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

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. 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 <artem.bityutskiy@linux.intel.com>
---
 drivers/idle/intel_idle.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Changelog
 * v2
   - Adjust commit message (feedback from Rui Zhang).

Comments

Zhang Rui April 20, 2023, 8:21 a.m. UTC | #1 
On Thu, 2023-04-20 at 09:47 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> 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. 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 <artem.bityutskiy@linux.intel.com>

Reviewed-by: Zhang Rui <rui.zhang@intel.com>

thanks,
rui
> ---
>  drivers/idle/intel_idle.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> Changelog
>  * v2
>    - Adjust commit message (feedback from Rui Zhang).
> 
> 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]))
diff mbox series

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]))