powercap/dtpm: move dtpm_subsys definition to dtpm.c

[PATCH] powercap/dtpm: move dtpm_subsys definition to dtpm.c

Posted by Tom Rix 4 years ago

Smatch reports this issue
dtpm_devfreq.c:200:24: warning: symbol 'dtpm_devfreq_ops'
  was not declared. Should it be static?

dtpm_devfreq_ops is declared in dtpm_subsys.h where it
is also used

extern struct dtpm_subsys_ops dtpm_devfreq_ops;
struct dtpm_subsys_ops *dtpm_subsys[] = {
...
	&dtpm_devfreq_ops,

Global variables should not be defined in header files.
This only works because dtpm.c is the only includer
of dtpm_subsys.h and user of dtpm_susys[].

Move the definition of dtpm_subsys[] to dtpm.c and change
the storage-class specifier to static.

Signed-off-by: Tom Rix <trix@redhat.com>
---
 drivers/powercap/dtpm.c        | 9 +++++++++
 drivers/powercap/dtpm_subsys.h | 9 ---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index ce920f17f45f..827a2509bec7 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -29,6 +29,15 @@
 
 #define DTPM_POWER_LIMIT_FLAG 0
 
+static struct dtpm_subsys_ops *dtpm_subsys[] = {
+#ifdef CONFIG_DTPM_CPU
+	&dtpm_cpu_ops,
+#endif
+#ifdef CONFIG_DTPM_DEVFREQ
+	&dtpm_devfreq_ops,
+#endif
+};
+
 static const char *constraint_name[] = {
 	"Instantaneous",
 };
diff --git a/drivers/powercap/dtpm_subsys.h b/drivers/powercap/dtpm_subsys.h
index db1712938a96..2db9a3efba93 100644
--- a/drivers/powercap/dtpm_subsys.h
+++ b/drivers/powercap/dtpm_subsys.h
@@ -10,13 +10,4 @@
 extern struct dtpm_subsys_ops dtpm_cpu_ops;
 extern struct dtpm_subsys_ops dtpm_devfreq_ops;
 
-struct dtpm_subsys_ops *dtpm_subsys[] = {
-#ifdef CONFIG_DTPM_CPU
-	&dtpm_cpu_ops,
-#endif
-#ifdef CONFIG_DTPM_DEVFREQ
-	&dtpm_devfreq_ops,
-#endif
-};
-
 #endif
-- 
2.27.0

Re: [PATCH] powercap/dtpm: move dtpm_subsys definition to dtpm.c

Posted by Rafael J. Wysocki 4 years ago

On Mon, Apr 18, 2022 at 8:08 PM Tom Rix <trix@redhat.com> wrote:
>
> Smatch reports this issue
> dtpm_devfreq.c:200:24: warning: symbol 'dtpm_devfreq_ops'
>   was not declared. Should it be static?
>
> dtpm_devfreq_ops is declared in dtpm_subsys.h where it
> is also used
>
> extern struct dtpm_subsys_ops dtpm_devfreq_ops;
> struct dtpm_subsys_ops *dtpm_subsys[] = {
> ...
>         &dtpm_devfreq_ops,
>
> Global variables should not be defined in header files.
> This only works because dtpm.c is the only includer
> of dtpm_subsys.h and user of dtpm_susys[].
>
> Move the definition of dtpm_subsys[] to dtpm.c and change
> the storage-class specifier to static.
>
> Signed-off-by: Tom Rix <trix@redhat.com>

Daniel, any comments?

Or do you want to pick it up yourself?

> ---
>  drivers/powercap/dtpm.c        | 9 +++++++++
>  drivers/powercap/dtpm_subsys.h | 9 ---------
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index ce920f17f45f..827a2509bec7 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -29,6 +29,15 @@
>
>  #define DTPM_POWER_LIMIT_FLAG 0
>
> +static struct dtpm_subsys_ops *dtpm_subsys[] = {
> +#ifdef CONFIG_DTPM_CPU
> +       &dtpm_cpu_ops,
> +#endif
> +#ifdef CONFIG_DTPM_DEVFREQ
> +       &dtpm_devfreq_ops,
> +#endif
> +};
> +
>  static const char *constraint_name[] = {
>         "Instantaneous",
>  };
> diff --git a/drivers/powercap/dtpm_subsys.h b/drivers/powercap/dtpm_subsys.h
> index db1712938a96..2db9a3efba93 100644
> --- a/drivers/powercap/dtpm_subsys.h
> +++ b/drivers/powercap/dtpm_subsys.h
> @@ -10,13 +10,4 @@
>  extern struct dtpm_subsys_ops dtpm_cpu_ops;
>  extern struct dtpm_subsys_ops dtpm_devfreq_ops;
>
> -struct dtpm_subsys_ops *dtpm_subsys[] = {
> -#ifdef CONFIG_DTPM_CPU
> -       &dtpm_cpu_ops,
> -#endif
> -#ifdef CONFIG_DTPM_DEVFREQ
> -       &dtpm_devfreq_ops,
> -#endif
> -};
> -
>  #endif
> --
> 2.27.0
>

Re: [PATCH] powercap/dtpm: move dtpm_subsys definition to dtpm.c

Posted by Daniel Lezcano 4 years ago

On 22/04/2022 15:56, Rafael J. Wysocki wrote:
> On Mon, Apr 18, 2022 at 8:08 PM Tom Rix <trix@redhat.com> wrote:
>>
>> Smatch reports this issue
>> dtpm_devfreq.c:200:24: warning: symbol 'dtpm_devfreq_ops'
>>    was not declared. Should it be static?
>>
>> dtpm_devfreq_ops is declared in dtpm_subsys.h where it
>> is also used
>>
>> extern struct dtpm_subsys_ops dtpm_devfreq_ops;
>> struct dtpm_subsys_ops *dtpm_subsys[] = {
>> ...
>>          &dtpm_devfreq_ops,
>>
>> Global variables should not be defined in header files.
>> This only works because dtpm.c is the only includer
>> of dtpm_subsys.h and user of dtpm_susys[].
>>
>> Move the definition of dtpm_subsys[] to dtpm.c and change
>> the storage-class specifier to static.
>>
>> Signed-off-by: Tom Rix <trix@redhat.com>
> 
> Daniel, any comments?
> 
> Or do you want to pick it up yourself?

Yes, I'll pick it

Thanks


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog