2022-04-23 21:05:42

by Tom Rix

Subject: [PATCH] platform/x86/intel: pmc/core: change pmc_lpm_modes to static

Sparse reports this issue
core.c: note: in included file:
core.h:239:12: warning: symbol 'pmc_lpm_modes' was not declared. Should it be static?

Global variables should not be defined in headers. This only works
because core.h is only included by core.c. Single file use
variables should be static, so change its storage-class specifier
to static.

Signed-off-by: Tom Rix <[email protected]>
---
drivers/platform/x86/intel/pmc/core.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index a46d3b53bf61..7a059e02c265 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -236,7 +236,7 @@ enum ppfear_regs {
#define ADL_LPM_STATUS_LATCH_EN_OFFSET 0x1704
#define ADL_LPM_LIVE_STATUS_OFFSET 0x1764

-const char *pmc_lpm_modes[] = {
+static const char *pmc_lpm_modes[] = {
"S0i2.0",
"S0i2.1",
"S0i2.2",
--
2.27.0

2022-04-26 06:35:16

by David E. Box

Subject: Re: [PATCH] platform/x86/intel: pmc/core: change pmc_lpm_modes to static

On Sat, 2022-04-23 at 08:30 -0400, Tom Rix wrote:
> Sparse reports this issue
> core.c: note: in included file:
> core.h:239:12: warning: symbol 'pmc_lpm_modes' was not declared. Should it be
> static?
>
> Global variables should not be defined in headers. This only works
> because core.h is only included by core.c. Single file use
> variables should be static, so change its storage-class specifier
> to static.
>
> Signed-off-by: Tom Rix <[email protected]>

Reviewed-by: David E. Box <[email protected]>

Thanks

> ---
> drivers/platform/x86/intel/pmc/core.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h
> b/drivers/platform/x86/intel/pmc/core.h
> index a46d3b53bf61..7a059e02c265 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -236,7 +236,7 @@ enum ppfear_regs {
> #define ADL_LPM_STATUS_LATCH_EN_OFFSET 0x1704
> #define ADL_LPM_LIVE_STATUS_OFFSET 0x1764
>
> -const char *pmc_lpm_modes[] = {
> +static const char *pmc_lpm_modes[] = {
> "S0i2.0",
> "S0i2.1",
> "S0i2.2",

2022-04-27 15:12:27

by Hans de Goede

Subject: Re: [PATCH] platform/x86/intel: pmc/core: change pmc_lpm_modes to static

Hi,

On 4/23/22 14:30, Tom Rix wrote:
> Sparse reports this issue
> core.c: note: in included file:
> core.h:239:12: warning: symbol 'pmc_lpm_modes' was not declared. Should it be static?
>
> Global variables should not be defined in headers. This only works
> because core.h is only included by core.c. Single file use
> variables should be static, so change its storage-class specifier
> to static.
>
> Signed-off-by: Tom Rix <[email protected]>

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> ---
> drivers/platform/x86/intel/pmc/core.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index a46d3b53bf61..7a059e02c265 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -236,7 +236,7 @@ enum ppfear_regs {
> #define ADL_LPM_STATUS_LATCH_EN_OFFSET 0x1704
> #define ADL_LPM_LIVE_STATUS_OFFSET 0x1764
>
> -const char *pmc_lpm_modes[] = {
> +static const char *pmc_lpm_modes[] = {
> "S0i2.0",
> "S0i2.1",
> "S0i2.2",