* [PATCH] intel_idle: remove a couple of useless variables
@ 2021-09-24 7:59 Artem Bityutskiy
2021-09-24 10:45 ` [PATCH v2] " Artem Bityutskiy
0 siblings, 1 reply; 4+ messages in thread
From: Artem Bityutskiy @ 2021-09-24 7:59 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM Mailing List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
This patch is a cleanup and has no functional impact.
Remove the 'auto_demotion_disable_flags' and 'disable_promotion_to_c1e' global
variables because we already have them in the global 'icpu' structure. Other
'icpu' members like 'use_acpi' are not duplicated, and it's better to be
consistent on this. Consistency improves readability.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
drivers/idle/intel_idle.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 70c2237a7261..eb28c3059351 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -67,9 +67,6 @@ static unsigned int disabled_states_mask;
static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
-static unsigned long auto_demotion_disable_flags;
-static bool disable_promotion_to_c1e;
-
struct idle_cpu {
struct cpuidle_state *state_table;
@@ -1644,7 +1641,7 @@ static void auto_demotion_disable(void)
unsigned long long msr_bits;
rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
- msr_bits &= ~auto_demotion_disable_flags;
+ msr_bits &= ~icpu->auto_demotion_disable_flags;
wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
}
@@ -1676,10 +1673,10 @@ static int intel_idle_cpu_init(unsigned int cpu)
return -EIO;
}
- if (auto_demotion_disable_flags)
+ if (icpu->auto_demotion_disable_flags)
auto_demotion_disable();
- if (disable_promotion_to_c1e)
+ if (icpu->disable_promotion_to_c1e)
c1e_promotion_disable();
return 0;
@@ -1757,8 +1754,6 @@ static int __init intel_idle_init(void)
icpu = (const struct idle_cpu *)id->driver_data;
if (icpu) {
cpuidle_state_table = icpu->state_table;
- auto_demotion_disable_flags = icpu->auto_demotion_disable_flags;
- disable_promotion_to_c1e = icpu->disable_promotion_to_c1e;
if (icpu->use_acpi || force_use_acpi)
intel_idle_acpi_cst_extract();
} else if (!intel_idle_acpi_cst_extract()) {
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2] intel_idle: remove a couple of useless variables
2021-09-24 7:59 [PATCH] intel_idle: remove a couple of useless variables Artem Bityutskiy
@ 2021-09-24 10:45 ` Artem Bityutskiy
2021-09-24 18:19 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Artem Bityutskiy @ 2021-09-24 10:45 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM Mailing List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
This patch is a cleanup and has no functional impact.
Remove the 'auto_demotion_disable_flags' and 'disable_promotion_to_c1e' global
variables because we already have them in the global 'icpu' structure. Other
'icpu' members like 'use_acpi' are not duplicated, and it's better to be
consistent on this. Consistency improves readability.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
Changes since v1:
* fixed crash on a platform for which the driver does not have a table. In
this case 'icpu' is NULL and we need to check for NULL in
'intel_idle_cpu_init()'.
drivers/idle/intel_idle.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 70c2237a7261..a73183016dd4 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -67,9 +67,6 @@ static unsigned int disabled_states_mask;
static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
-static unsigned long auto_demotion_disable_flags;
-static bool disable_promotion_to_c1e;
-
struct idle_cpu {
struct cpuidle_state *state_table;
@@ -1644,7 +1641,7 @@ static void auto_demotion_disable(void)
unsigned long long msr_bits;
rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
- msr_bits &= ~auto_demotion_disable_flags;
+ msr_bits &= ~icpu->auto_demotion_disable_flags;
wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
}
@@ -1676,10 +1673,13 @@ static int intel_idle_cpu_init(unsigned int cpu)
return -EIO;
}
- if (auto_demotion_disable_flags)
+ if (!icpu)
+ return 0;
+
+ if (icpu->auto_demotion_disable_flags)
auto_demotion_disable();
- if (disable_promotion_to_c1e)
+ if (icpu->disable_promotion_to_c1e)
c1e_promotion_disable();
return 0;
@@ -1757,8 +1757,6 @@ static int __init intel_idle_init(void)
icpu = (const struct idle_cpu *)id->driver_data;
if (icpu) {
cpuidle_state_table = icpu->state_table;
- auto_demotion_disable_flags = icpu->auto_demotion_disable_flags;
- disable_promotion_to_c1e = icpu->disable_promotion_to_c1e;
if (icpu->use_acpi || force_use_acpi)
intel_idle_acpi_cst_extract();
} else if (!intel_idle_acpi_cst_extract()) {
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] intel_idle: remove a couple of useless variables
2021-09-24 10:45 ` [PATCH v2] " Artem Bityutskiy
@ 2021-09-24 18:19 ` Rafael J. Wysocki
2021-09-27 6:52 ` Artem Bityutskiy
0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2021-09-24 18:19 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: Rafael J. Wysocki, Linux PM Mailing List
On Fri, Sep 24, 2021 at 12:45 PM Artem Bityutskiy <dedekind1@gmail.com> wrote:
>
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> This patch is a cleanup and has no functional impact.
Are you sure?
> Remove the 'auto_demotion_disable_flags' and 'disable_promotion_to_c1e' global
> variables because we already have them in the global 'icpu' structure.
Which is __initdata and these variables aren't.
> Other 'icpu' members like 'use_acpi' are not duplicated,
Because it is not necessary to access them after initialization.
> and it's better to be consistent on this.
Well, the code is as consistent as it can be.
>Consistency improves readability.
That's true, but it has no bearing in this particular case IMO.
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>
> Changes since v1:
> * fixed crash on a platform for which the driver does not have a table. In
> this case 'icpu' is NULL and we need to check for NULL in
> 'intel_idle_cpu_init()'.
>
> drivers/idle/intel_idle.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 70c2237a7261..a73183016dd4 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -67,9 +67,6 @@ static unsigned int disabled_states_mask;
>
> static struct cpuidle_device __percpu *intel_idle_cpuidle_devices;
>
> -static unsigned long auto_demotion_disable_flags;
> -static bool disable_promotion_to_c1e;
> -
> struct idle_cpu {
> struct cpuidle_state *state_table;
>
> @@ -1644,7 +1641,7 @@ static void auto_demotion_disable(void)
> unsigned long long msr_bits;
>
> rdmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
> - msr_bits &= ~auto_demotion_disable_flags;
> + msr_bits &= ~icpu->auto_demotion_disable_flags;
> wrmsrl(MSR_PKG_CST_CONFIG_CONTROL, msr_bits);
> }
>
> @@ -1676,10 +1673,13 @@ static int intel_idle_cpu_init(unsigned int cpu)
> return -EIO;
> }
>
> - if (auto_demotion_disable_flags)
> + if (!icpu)
> + return 0;
> +
> + if (icpu->auto_demotion_disable_flags)
> auto_demotion_disable();
>
> - if (disable_promotion_to_c1e)
> + if (icpu->disable_promotion_to_c1e)
> c1e_promotion_disable();
>
> return 0;
> @@ -1757,8 +1757,6 @@ static int __init intel_idle_init(void)
> icpu = (const struct idle_cpu *)id->driver_data;
> if (icpu) {
> cpuidle_state_table = icpu->state_table;
> - auto_demotion_disable_flags = icpu->auto_demotion_disable_flags;
> - disable_promotion_to_c1e = icpu->disable_promotion_to_c1e;
> if (icpu->use_acpi || force_use_acpi)
> intel_idle_acpi_cst_extract();
> } else if (!intel_idle_acpi_cst_extract()) {
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] intel_idle: remove a couple of useless variables
2021-09-24 18:19 ` Rafael J. Wysocki
@ 2021-09-27 6:52 ` Artem Bityutskiy
0 siblings, 0 replies; 4+ messages in thread
From: Artem Bityutskiy @ 2021-09-27 6:52 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM Mailing List
On Fri, 2021-09-24 at 20:19 +0200, Rafael J. Wysocki wrote:
> On Fri, Sep 24, 2021 at 12:45 PM Artem Bityutskiy <dedekind1@gmail.com> wrote:
> >
> > From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> >
> > This patch is a cleanup and has no functional impact.
>
> Are you sure?
Not anymore. I shouldn't have sent this patch out so quickly, sorry.
Thanks for review.
Artem.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-27 6:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-24 7:59 [PATCH] intel_idle: remove a couple of useless variables Artem Bityutskiy
2021-09-24 10:45 ` [PATCH v2] " Artem Bityutskiy
2021-09-24 18:19 ` Rafael J. Wysocki
2021-09-27 6:52 ` Artem Bityutskiy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.