2024-05-28 12:11:42

by Arnd Bergmann

Subject: [PATCH] cpufreq: amd-pstate: remove global header file

From: Arnd Bergmann <[email protected]>

When extra warnings are enabled, gcc points out a global variable
definition in a header:

In file included from drivers/cpufreq/amd-pstate-ut.c:29:
include/linux/amd-pstate.h:123:27: error: 'amd_pstate_mode_string' defined but not used [-Werror=unused-const-variable=]
123 | static const char * const amd_pstate_mode_string[] = {
| ^~~~~~~~~~~~~~~~~~~~~~

This header is only included from two files in the same directory,
and one of them uses only a single definition from it, so clean it
up by moving most of the contents into the driver that uses them,
and making shared bits a local header file.

Fixes: 36c5014e5460 ("cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param()")
Signed-off-by: Arnd Bergmann <[email protected]>
---
MAINTAINERS | 1 -
drivers/cpufreq/amd-pstate-ut.c | 3 +-
drivers/cpufreq/amd-pstate.c | 34 ++++++++++++++++++-
.../linux => drivers/cpufreq}/amd-pstate.h | 33 ------------------
4 files changed, 35 insertions(+), 36 deletions(-)
rename {include/linux => drivers/cpufreq}/amd-pstate.h (82%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 070a39b2b098..35a75ab8ef05 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1107,7 +1107,6 @@ L: [email protected]
S: Supported
F: Documentation/admin-guide/pm/amd-pstate.rst
F: drivers/cpufreq/amd-pstate*
-F: include/linux/amd-pstate.h
F: tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py

AMD PTDMA DRIVER
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index f04ae67dda37..fc275d41d51e 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -26,10 +26,11 @@
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/fs.h>
-#include <linux/amd-pstate.h>

#include <acpi/cppc_acpi.h>

+#include "amd-pstate.h"
+
/*
* Abbreviations:
* amd_pstate_ut: used as a shortform for AMD P-State unit test.
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 1b7e82a0ad2e..91993647e09e 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -36,7 +36,6 @@
#include <linux/delay.h>
#include <linux/uaccess.h>
#include <linux/static_call.h>
-#include <linux/amd-pstate.h>
#include <linux/topology.h>

#include <acpi/processor.h>
@@ -46,6 +45,8 @@
#include <asm/processor.h>
#include <asm/cpufeature.h>
#include <asm/cpu_device_id.h>
+
+#include "amd-pstate.h"
#include "amd-pstate-trace.h"

#define AMD_PSTATE_TRANSITION_LATENCY 20000
@@ -53,6 +54,37 @@
#define CPPC_HIGHEST_PERF_PERFORMANCE 196
#define CPPC_HIGHEST_PERF_DEFAULT 166

+#define AMD_CPPC_EPP_PERFORMANCE 0x00
+#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
+#define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF
+#define AMD_CPPC_EPP_POWERSAVE 0xFF
+
+/*
+ * enum amd_pstate_mode - driver working mode of amd pstate
+ */
+enum amd_pstate_mode {
+ AMD_PSTATE_UNDEFINED = 0,
+ AMD_PSTATE_DISABLE,
+ AMD_PSTATE_PASSIVE,
+ AMD_PSTATE_ACTIVE,
+ AMD_PSTATE_GUIDED,
+ AMD_PSTATE_MAX,
+};
+
+static const char * const amd_pstate_mode_string[] = {
+ [AMD_PSTATE_UNDEFINED] = "undefined",
+ [AMD_PSTATE_DISABLE] = "disable",
+ [AMD_PSTATE_PASSIVE] = "passive",
+ [AMD_PSTATE_ACTIVE] = "active",
+ [AMD_PSTATE_GUIDED] = "guided",
+ NULL,
+};
+
+struct quirk_entry {
+ u32 nominal_freq;
+ u32 lowest_freq;
+};
+
/*
* TODO: We need more time to fine tune processors with shared memory solution
* with community together.
diff --git a/include/linux/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
similarity index 82%
rename from include/linux/amd-pstate.h
rename to drivers/cpufreq/amd-pstate.h
index d58fc022ec46..e6a28e7f4dbf 100644
--- a/include/linux/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -1,7 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * linux/include/linux/amd-pstate.h
- *
* Copyright (C) 2022 Advanced Micro Devices, Inc.
*
* Author: Meng Li <[email protected]>
@@ -12,11 +10,6 @@

#include <linux/pm_qos.h>

-#define AMD_CPPC_EPP_PERFORMANCE 0x00
-#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
-#define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF
-#define AMD_CPPC_EPP_POWERSAVE 0xFF
-
/*********************************************************************
* AMD P-state INTERFACE *
*********************************************************************/
@@ -108,30 +101,4 @@ struct amd_cpudata {
bool suspended;
};

-/*
- * enum amd_pstate_mode - driver working mode of amd pstate
- */
-enum amd_pstate_mode {
- AMD_PSTATE_UNDEFINED = 0,
- AMD_PSTATE_DISABLE,
- AMD_PSTATE_PASSIVE,
- AMD_PSTATE_ACTIVE,
- AMD_PSTATE_GUIDED,
- AMD_PSTATE_MAX,
-};
-
-static const char * const amd_pstate_mode_string[] = {
- [AMD_PSTATE_UNDEFINED] = "undefined",
- [AMD_PSTATE_DISABLE] = "disable",
- [AMD_PSTATE_PASSIVE] = "passive",
- [AMD_PSTATE_ACTIVE] = "active",
- [AMD_PSTATE_GUIDED] = "guided",
- NULL,
-};
-
-struct quirk_entry {
- u32 nominal_freq;
- u32 lowest_freq;
-};
-
#endif /* _LINUX_AMD_PSTATE_H */
--
2.39.2

2024-05-28 17:49:48

by Mario Limonciello

Subject: Re: [PATCH] cpufreq: amd-pstate: remove global header file

On 5/28/2024 07:09, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> When extra warnings are enabled, gcc points out a global variable
> definition in a header:
>
> In file included from drivers/cpufreq/amd-pstate-ut.c:29:
> include/linux/amd-pstate.h:123:27: error: 'amd_pstate_mode_string' defined but not used [-Werror=unused-const-variable=]
> 123 | static const char * const amd_pstate_mode_string[] = {
> | ^~~~~~~~~~~~~~~~~~~~~~
>
> This header is only included from two files in the same directory,
> and one of them uses only a single definition from it, so clean it
> up by moving most of the contents into the driver that uses them,
> and making shared bits a local header file.
>
> Fixes: 36c5014e5460 ("cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param()")
> Signed-off-by: Arnd Bergmann <[email protected]>

Thanks!

Acked-by: Mario Limonciello <[email protected]>

> ---
> MAINTAINERS | 1 -
> drivers/cpufreq/amd-pstate-ut.c | 3 +-
> drivers/cpufreq/amd-pstate.c | 34 ++++++++++++++++++-
> .../linux => drivers/cpufreq}/amd-pstate.h | 33 ------------------
> 4 files changed, 35 insertions(+), 36 deletions(-)
> rename {include/linux => drivers/cpufreq}/amd-pstate.h (82%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 070a39b2b098..35a75ab8ef05 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1107,7 +1107,6 @@ L: [email protected]
> S: Supported
> F: Documentation/admin-guide/pm/amd-pstate.rst
> F: drivers/cpufreq/amd-pstate*
> -F: include/linux/amd-pstate.h
> F: tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
>
> AMD PTDMA DRIVER
> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> index f04ae67dda37..fc275d41d51e 100644
> --- a/drivers/cpufreq/amd-pstate-ut.c
> +++ b/drivers/cpufreq/amd-pstate-ut.c
> @@ -26,10 +26,11 @@
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/fs.h>
> -#include <linux/amd-pstate.h>
>
> #include <acpi/cppc_acpi.h>
>
> +#include "amd-pstate.h"
> +
> /*
> * Abbreviations:
> * amd_pstate_ut: used as a shortform for AMD P-State unit test.
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 1b7e82a0ad2e..91993647e09e 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -36,7 +36,6 @@
> #include <linux/delay.h>
> #include <linux/uaccess.h>
> #include <linux/static_call.h>
> -#include <linux/amd-pstate.h>
> #include <linux/topology.h>
>
> #include <acpi/processor.h>
> @@ -46,6 +45,8 @@
> #include <asm/processor.h>
> #include <asm/cpufeature.h>
> #include <asm/cpu_device_id.h>
> +
> +#include "amd-pstate.h"
> #include "amd-pstate-trace.h"
>
> #define AMD_PSTATE_TRANSITION_LATENCY 20000
> @@ -53,6 +54,37 @@
> #define CPPC_HIGHEST_PERF_PERFORMANCE 196
> #define CPPC_HIGHEST_PERF_DEFAULT 166
>
> +#define AMD_CPPC_EPP_PERFORMANCE 0x00
> +#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
> +#define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF
> +#define AMD_CPPC_EPP_POWERSAVE 0xFF
> +
> +/*
> + * enum amd_pstate_mode - driver working mode of amd pstate
> + */
> +enum amd_pstate_mode {
> + AMD_PSTATE_UNDEFINED = 0,
> + AMD_PSTATE_DISABLE,
> + AMD_PSTATE_PASSIVE,
> + AMD_PSTATE_ACTIVE,
> + AMD_PSTATE_GUIDED,
> + AMD_PSTATE_MAX,
> +};
> +
> +static const char * const amd_pstate_mode_string[] = {
> + [AMD_PSTATE_UNDEFINED] = "undefined",
> + [AMD_PSTATE_DISABLE] = "disable",
> + [AMD_PSTATE_PASSIVE] = "passive",
> + [AMD_PSTATE_ACTIVE] = "active",
> + [AMD_PSTATE_GUIDED] = "guided",
> + NULL,
> +};
> +
> +struct quirk_entry {
> + u32 nominal_freq;
> + u32 lowest_freq;
> +};
> +
> /*
> * TODO: We need more time to fine tune processors with shared memory solution
> * with community together.
> diff --git a/include/linux/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> similarity index 82%
> rename from include/linux/amd-pstate.h
> rename to drivers/cpufreq/amd-pstate.h
> index d58fc022ec46..e6a28e7f4dbf 100644
> --- a/include/linux/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -1,7 +1,5 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> /*
> - * linux/include/linux/amd-pstate.h
> - *
> * Copyright (C) 2022 Advanced Micro Devices, Inc.
> *
> * Author: Meng Li <[email protected]>
> @@ -12,11 +10,6 @@
>
> #include <linux/pm_qos.h>
>
> -#define AMD_CPPC_EPP_PERFORMANCE 0x00
> -#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
> -#define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF
> -#define AMD_CPPC_EPP_POWERSAVE 0xFF
> -
> /*********************************************************************
> * AMD P-state INTERFACE *
> *********************************************************************/
> @@ -108,30 +101,4 @@ struct amd_cpudata {
> bool suspended;
> };
>
> -/*
> - * enum amd_pstate_mode - driver working mode of amd pstate
> - */
> -enum amd_pstate_mode {
> - AMD_PSTATE_UNDEFINED = 0,
> - AMD_PSTATE_DISABLE,
> - AMD_PSTATE_PASSIVE,
> - AMD_PSTATE_ACTIVE,
> - AMD_PSTATE_GUIDED,
> - AMD_PSTATE_MAX,
> -};
> -
> -static const char * const amd_pstate_mode_string[] = {
> - [AMD_PSTATE_UNDEFINED] = "undefined",
> - [AMD_PSTATE_DISABLE] = "disable",
> - [AMD_PSTATE_PASSIVE] = "passive",
> - [AMD_PSTATE_ACTIVE] = "active",
> - [AMD_PSTATE_GUIDED] = "guided",
> - NULL,
> -};
> -
> -struct quirk_entry {
> - u32 nominal_freq;
> - u32 lowest_freq;
> -};
> -
> #endif /* _LINUX_AMD_PSTATE_H */

2024-05-28 20:01:02

by Rafael J. Wysocki

Subject: Re: [PATCH] cpufreq: amd-pstate: remove global header file

On Tue, May 28, 2024 at 7:49 PM Mario Limonciello
<[email protected]> wrote:
>
> On 5/28/2024 07:09, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > When extra warnings are enabled, gcc points out a global variable
> > definition in a header:
> >
> > In file included from drivers/cpufreq/amd-pstate-ut.c:29:
> > include/linux/amd-pstate.h:123:27: error: 'amd_pstate_mode_string' defined but not used [-Werror=unused-const-variable=]
> > 123 | static const char * const amd_pstate_mode_string[] = {
> > | ^~~~~~~~~~~~~~~~~~~~~~
> >
> > This header is only included from two files in the same directory,
> > and one of them uses only a single definition from it, so clean it
> > up by moving most of the contents into the driver that uses them,
> > and making shared bits a local header file.
> >
> > Fixes: 36c5014e5460 ("cpufreq: amd-pstate: optimize driver working mode selection in amd_pstate_param()")
> > Signed-off-by: Arnd Bergmann <[email protected]>
>
> Thanks!
>
> Acked-by: Mario Limonciello <[email protected]>

Applied as 6.10-rc material, thanks!

> > ---
> > MAINTAINERS | 1 -
> > drivers/cpufreq/amd-pstate-ut.c | 3 +-
> > drivers/cpufreq/amd-pstate.c | 34 ++++++++++++++++++-
> > .../linux => drivers/cpufreq}/amd-pstate.h | 33 ------------------
> > 4 files changed, 35 insertions(+), 36 deletions(-)
> > rename {include/linux => drivers/cpufreq}/amd-pstate.h (82%)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 070a39b2b098..35a75ab8ef05 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1107,7 +1107,6 @@ L: [email protected]
> > S: Supported
> > F: Documentation/admin-guide/pm/amd-pstate.rst
> > F: drivers/cpufreq/amd-pstate*
> > -F: include/linux/amd-pstate.h
> > F: tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
> >
> > AMD PTDMA DRIVER
> > diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
> > index f04ae67dda37..fc275d41d51e 100644
> > --- a/drivers/cpufreq/amd-pstate-ut.c
> > +++ b/drivers/cpufreq/amd-pstate-ut.c
> > @@ -26,10 +26,11 @@
> > #include <linux/module.h>
> > #include <linux/moduleparam.h>
> > #include <linux/fs.h>
> > -#include <linux/amd-pstate.h>
> >
> > #include <acpi/cppc_acpi.h>
> >
> > +#include "amd-pstate.h"
> > +
> > /*
> > * Abbreviations:
> > * amd_pstate_ut: used as a shortform for AMD P-State unit test.
> > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> > index 1b7e82a0ad2e..91993647e09e 100644
> > --- a/drivers/cpufreq/amd-pstate.c
> > +++ b/drivers/cpufreq/amd-pstate.c
> > @@ -36,7 +36,6 @@
> > #include <linux/delay.h>
> > #include <linux/uaccess.h>
> > #include <linux/static_call.h>
> > -#include <linux/amd-pstate.h>
> > #include <linux/topology.h>
> >
> > #include <acpi/processor.h>
> > @@ -46,6 +45,8 @@
> > #include <asm/processor.h>
> > #include <asm/cpufeature.h>
> > #include <asm/cpu_device_id.h>
> > +
> > +#include "amd-pstate.h"
> > #include "amd-pstate-trace.h"
> >
> > #define AMD_PSTATE_TRANSITION_LATENCY 20000
> > @@ -53,6 +54,37 @@
> > #define CPPC_HIGHEST_PERF_PERFORMANCE 196
> > #define CPPC_HIGHEST_PERF_DEFAULT 166
> >
> > +#define AMD_CPPC_EPP_PERFORMANCE 0x00
> > +#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
> > +#define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF
> > +#define AMD_CPPC_EPP_POWERSAVE 0xFF
> > +
> > +/*
> > + * enum amd_pstate_mode - driver working mode of amd pstate
> > + */
> > +enum amd_pstate_mode {
> > + AMD_PSTATE_UNDEFINED = 0,
> > + AMD_PSTATE_DISABLE,
> > + AMD_PSTATE_PASSIVE,
> > + AMD_PSTATE_ACTIVE,
> > + AMD_PSTATE_GUIDED,
> > + AMD_PSTATE_MAX,
> > +};
> > +
> > +static const char * const amd_pstate_mode_string[] = {
> > + [AMD_PSTATE_UNDEFINED] = "undefined",
> > + [AMD_PSTATE_DISABLE] = "disable",
> > + [AMD_PSTATE_PASSIVE] = "passive",
> > + [AMD_PSTATE_ACTIVE] = "active",
> > + [AMD_PSTATE_GUIDED] = "guided",
> > + NULL,
> > +};
> > +
> > +struct quirk_entry {
> > + u32 nominal_freq;
> > + u32 lowest_freq;
> > +};
> > +
> > /*
> > * TODO: We need more time to fine tune processors with shared memory solution
> > * with community together.
> > diff --git a/include/linux/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> > similarity index 82%
> > rename from include/linux/amd-pstate.h
> > rename to drivers/cpufreq/amd-pstate.h
> > index d58fc022ec46..e6a28e7f4dbf 100644
> > --- a/include/linux/amd-pstate.h
> > +++ b/drivers/cpufreq/amd-pstate.h
> > @@ -1,7 +1,5 @@
> > /* SPDX-License-Identifier: GPL-2.0-only */
> > /*
> > - * linux/include/linux/amd-pstate.h
> > - *
> > * Copyright (C) 2022 Advanced Micro Devices, Inc.
> > *
> > * Author: Meng Li <[email protected]>
> > @@ -12,11 +10,6 @@
> >
> > #include <linux/pm_qos.h>
> >
> > -#define AMD_CPPC_EPP_PERFORMANCE 0x00
> > -#define AMD_CPPC_EPP_BALANCE_PERFORMANCE 0x80
> > -#define AMD_CPPC_EPP_BALANCE_POWERSAVE 0xBF
> > -#define AMD_CPPC_EPP_POWERSAVE 0xFF
> > -
> > /*********************************************************************
> > * AMD P-state INTERFACE *
> > *********************************************************************/
> > @@ -108,30 +101,4 @@ struct amd_cpudata {
> > bool suspended;
> > };
> >
> > -/*
> > - * enum amd_pstate_mode - driver working mode of amd pstate
> > - */
> > -enum amd_pstate_mode {
> > - AMD_PSTATE_UNDEFINED = 0,
> > - AMD_PSTATE_DISABLE,
> > - AMD_PSTATE_PASSIVE,
> > - AMD_PSTATE_ACTIVE,
> > - AMD_PSTATE_GUIDED,
> > - AMD_PSTATE_MAX,
> > -};
> > -
> > -static const char * const amd_pstate_mode_string[] = {
> > - [AMD_PSTATE_UNDEFINED] = "undefined",
> > - [AMD_PSTATE_DISABLE] = "disable",
> > - [AMD_PSTATE_PASSIVE] = "passive",
> > - [AMD_PSTATE_ACTIVE] = "active",
> > - [AMD_PSTATE_GUIDED] = "guided",
> > - NULL,
> > -};
> > -
> > -struct quirk_entry {
> > - u32 nominal_freq;
> > - u32 lowest_freq;
> > -};
> > -
> > #endif /* _LINUX_AMD_PSTATE_H */
>