Building the cppc_cpufreq driver with for arm64 with
CONFIG_ENERGY_MODEL=n triggers the following warnings:
drivers/cpufreq/cppc_cpufreq.c:550:12: error: ‘cppc_get_cpu_cost’ defined but not used
[-Werror=unused-function]
550 | static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
| ^~~~~~~~~~~~~~~~~
drivers/cpufreq/cppc_cpufreq.c:481:12: error: ‘cppc_get_cpu_power’ defined but not used
[-Werror=unused-function]
481 | static int cppc_get_cpu_power(struct device *cpu_dev,
| ^~~~~~~~~~~~~~~~~~
Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information")
Reported-by: Shaokun Zhang <[email protected]>
Signed-off-by: Pierre Gondois <[email protected]>
---
drivers/cpufreq/cppc_cpufreq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index d092c9bb4ba3..ecd0d3ee48c5 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -478,7 +478,7 @@ static inline unsigned long compute_cost(int cpu, int step)
step * CPPC_EM_COST_STEP;
}
-static int cppc_get_cpu_power(struct device *cpu_dev,
+static __maybe_unused int cppc_get_cpu_power(struct device *cpu_dev,
unsigned long *power, unsigned long *KHz)
{
unsigned long perf_step, perf_prev, perf, perf_check;
@@ -547,8 +547,8 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
return 0;
}
-static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
- unsigned long *cost)
+static __maybe_unused int cppc_get_cpu_cost(struct device *cpu_dev,
+ unsigned long KHz, unsigned long *cost)
{
unsigned long perf_step, perf_prev;
struct cppc_perf_caps *perf_caps;
--
2.25.1
On 30-05-22, 10:12, Pierre Gondois wrote:
> Building the cppc_cpufreq driver with for arm64 with
> CONFIG_ENERGY_MODEL=n triggers the following warnings:
> drivers/cpufreq/cppc_cpufreq.c:550:12: error: ‘cppc_get_cpu_cost’ defined but not used
> [-Werror=unused-function]
> 550 | static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
> | ^~~~~~~~~~~~~~~~~
> drivers/cpufreq/cppc_cpufreq.c:481:12: error: ‘cppc_get_cpu_power’ defined but not used
> [-Werror=unused-function]
> 481 | static int cppc_get_cpu_power(struct device *cpu_dev,
> | ^~~~~~~~~~~~~~~~~~
>
> Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information")
> Reported-by: Shaokun Zhang <[email protected]>
> Signed-off-by: Pierre Gondois <[email protected]>
> ---
> drivers/cpufreq/cppc_cpufreq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index d092c9bb4ba3..ecd0d3ee48c5 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -478,7 +478,7 @@ static inline unsigned long compute_cost(int cpu, int step)
> step * CPPC_EM_COST_STEP;
> }
>
> -static int cppc_get_cpu_power(struct device *cpu_dev,
> +static __maybe_unused int cppc_get_cpu_power(struct device *cpu_dev,
> unsigned long *power, unsigned long *KHz)
> {
> unsigned long perf_step, perf_prev, perf, perf_check;
> @@ -547,8 +547,8 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
> return 0;
> }
>
> -static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
> - unsigned long *cost)
> +static __maybe_unused int cppc_get_cpu_cost(struct device *cpu_dev,
> + unsigned long KHz, unsigned long *cost)
> {
> unsigned long perf_step, perf_prev;
> struct cppc_perf_caps *perf_caps;
Should we actually run cppc_cpufreq_register_em() for
!CONFIG_ENERGY_MODEL ? Why?
--
viresh
On 5/30/22 10:20, Viresh Kumar wrote:
> On 30-05-22, 10:12, Pierre Gondois wrote:
>> Building the cppc_cpufreq driver with for arm64 with
>> CONFIG_ENERGY_MODEL=n triggers the following warnings:
>> drivers/cpufreq/cppc_cpufreq.c:550:12: error: ‘cppc_get_cpu_cost’ defined but not used
>> [-Werror=unused-function]
>> 550 | static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
>> | ^~~~~~~~~~~~~~~~~
>> drivers/cpufreq/cppc_cpufreq.c:481:12: error: ‘cppc_get_cpu_power’ defined but not used
>> [-Werror=unused-function]
>> 481 | static int cppc_get_cpu_power(struct device *cpu_dev,
>> | ^~~~~~~~~~~~~~~~~~
>>
>> Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information")
>> Reported-by: Shaokun Zhang <[email protected]>
>> Signed-off-by: Pierre Gondois <[email protected]>
>> ---
>> drivers/cpufreq/cppc_cpufreq.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index d092c9bb4ba3..ecd0d3ee48c5 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -478,7 +478,7 @@ static inline unsigned long compute_cost(int cpu, int step)
>> step * CPPC_EM_COST_STEP;
>> }
>>
>> -static int cppc_get_cpu_power(struct device *cpu_dev,
>> +static __maybe_unused int cppc_get_cpu_power(struct device *cpu_dev,
>> unsigned long *power, unsigned long *KHz)
>> {
>> unsigned long perf_step, perf_prev, perf, perf_check;
>> @@ -547,8 +547,8 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
>> return 0;
>> }
>>
>> -static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
>> - unsigned long *cost)
>> +static __maybe_unused int cppc_get_cpu_cost(struct device *cpu_dev,
>> + unsigned long KHz, unsigned long *cost)
>> {
>> unsigned long perf_step, perf_prev;
>> struct cppc_perf_caps *perf_caps;
>
> Should we actually run cppc_cpufreq_register_em() for
> !CONFIG_ENERGY_MODEL ? Why?
>
Hello Viresh,
It seems that when CONFIG_ENERGY_MODEL=n, the compiler is already
considering cppc_cpufreq_register_em() as an empty function.
Indeed, CONFIG_ENERGY_MODEL=n makes em_dev_register_perf_domain()
an empty function, so cppc_cpufreq_register_em() is only made of
variable definitions. This compiler optimization also explains
why cppc_get_cpu_power() and cppc_get_cpu_cost() trigger the
-Wunused-function warning.
Putting cppc_cpufreq_register_em() inside an
#ifdef CONFIG_ENERGY_MODEL
guard seems also valid to me. To avoid too many empty definitions
of cppc_cpufreq_register_em(), I guess it should be inside an
#if defined(CONFIG_ARM64) && defined(CONFIG_ENERGY_MODEL)
guard instead.
Please let me know what you prefer.
Regards,
Pierre
On 5/30/22 11:07, Viresh Kumar wrote:
> On 30-05-22, 10:44, Pierre Gondois wrote:
>>
>>
>> On 5/30/22 10:20, Viresh Kumar wrote:
>>> On 30-05-22, 10:12, Pierre Gondois wrote:
>>>> Building the cppc_cpufreq driver with for arm64 with
>>>> CONFIG_ENERGY_MODEL=n triggers the following warnings:
>>>> drivers/cpufreq/cppc_cpufreq.c:550:12: error: ‘cppc_get_cpu_cost’ defined but not used
>>>> [-Werror=unused-function]
>>>> 550 | static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
>>>> | ^~~~~~~~~~~~~~~~~
>>>> drivers/cpufreq/cppc_cpufreq.c:481:12: error: ‘cppc_get_cpu_power’ defined but not used
>>>> [-Werror=unused-function]
>>>> 481 | static int cppc_get_cpu_power(struct device *cpu_dev,
>>>> | ^~~~~~~~~~~~~~~~~~
>>>>
>>>> Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information")
>>>> Reported-by: Shaokun Zhang <[email protected]>
>>>> Signed-off-by: Pierre Gondois <[email protected]>
>>>> ---
>>>> drivers/cpufreq/cppc_cpufreq.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>>> index d092c9bb4ba3..ecd0d3ee48c5 100644
>>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>>> @@ -478,7 +478,7 @@ static inline unsigned long compute_cost(int cpu, int step)
>>>> step * CPPC_EM_COST_STEP;
>>>> }
>>>> -static int cppc_get_cpu_power(struct device *cpu_dev,
>>>> +static __maybe_unused int cppc_get_cpu_power(struct device *cpu_dev,
>>>> unsigned long *power, unsigned long *KHz)
>>>> {
>>>> unsigned long perf_step, perf_prev, perf, perf_check;
>>>> @@ -547,8 +547,8 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
>>>> return 0;
>>>> }
>>>> -static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
>>>> - unsigned long *cost)
>>>> +static __maybe_unused int cppc_get_cpu_cost(struct device *cpu_dev,
>>>> + unsigned long KHz, unsigned long *cost)
>>>> {
>>>> unsigned long perf_step, perf_prev;
>>>> struct cppc_perf_caps *perf_caps;
>>>
>>> Should we actually run cppc_cpufreq_register_em() for
>>> !CONFIG_ENERGY_MODEL ? Why?
>>>
>>
>> Hello Viresh,
>> It seems that when CONFIG_ENERGY_MODEL=n, the compiler is already
>> considering cppc_cpufreq_register_em() as an empty function.
>>
>> Indeed, CONFIG_ENERGY_MODEL=n makes em_dev_register_perf_domain()
>> an empty function, so cppc_cpufreq_register_em() is only made of
>> variable definitions. This compiler optimization also explains
>> why cppc_get_cpu_power() and cppc_get_cpu_cost() trigger the
>> -Wunused-function warning.
>>
>> Putting cppc_cpufreq_register_em() inside an
>> #ifdef CONFIG_ENERGY_MODEL
>> guard seems also valid to me. To avoid too many empty definitions
>> of cppc_cpufreq_register_em(), I guess it should be inside an
>> #if defined(CONFIG_ARM64) && defined(CONFIG_ENERGY_MODEL)
>> guard instead.
>> Please let me know what you prefer.
>
> In that case we shouldn't do:
>
> cppc_cpufreq_driver.register_em = cppc_cpufreq_register_em;
>
> as well, as that is extra work for the cpufreq core, which won't be
> used at all.
>
> So instead of __maybe_unused, lets put all dependent stuff within
> CONFIG_ENERGY_MODEL ?
>
Ok yes. Just to be sure and except if disagreed, I will use the
following structure:
#if CONFIG_ARM64
#else
#endif
#if defined(CONFIG_ARM64) && defined(CONFIG_ENERGY_MODEL)
int populate_efficiency_class();
#else
int populate_efficiency_class();
#endif
to avoid having multiple empty definitions of
populate_efficiency_class() (for eg.) that we would have with:
#if CONFIG_ARM64
#if CONFIG_ENERGY_MODEL
int populate_efficiency_class();
#else // CONFIG_ENERGY_MODEL
int populate_efficiency_class();
#endif // CONFIG_ENERGY_MODEL
#else // CONFIG_ARM64
int populate_efficiency_class();
#endif // CONFIG_ARM64
On 30-05-22, 10:44, Pierre Gondois wrote:
>
>
> On 5/30/22 10:20, Viresh Kumar wrote:
> > On 30-05-22, 10:12, Pierre Gondois wrote:
> > > Building the cppc_cpufreq driver with for arm64 with
> > > CONFIG_ENERGY_MODEL=n triggers the following warnings:
> > > drivers/cpufreq/cppc_cpufreq.c:550:12: error: ‘cppc_get_cpu_cost’ defined but not used
> > > [-Werror=unused-function]
> > > 550 | static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
> > > | ^~~~~~~~~~~~~~~~~
> > > drivers/cpufreq/cppc_cpufreq.c:481:12: error: ‘cppc_get_cpu_power’ defined but not used
> > > [-Werror=unused-function]
> > > 481 | static int cppc_get_cpu_power(struct device *cpu_dev,
> > > | ^~~~~~~~~~~~~~~~~~
> > >
> > > Fixes: 740fcdc2c20e ("cpufreq: CPPC: Register EM based on efficiency class information")
> > > Reported-by: Shaokun Zhang <[email protected]>
> > > Signed-off-by: Pierre Gondois <[email protected]>
> > > ---
> > > drivers/cpufreq/cppc_cpufreq.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> > > index d092c9bb4ba3..ecd0d3ee48c5 100644
> > > --- a/drivers/cpufreq/cppc_cpufreq.c
> > > +++ b/drivers/cpufreq/cppc_cpufreq.c
> > > @@ -478,7 +478,7 @@ static inline unsigned long compute_cost(int cpu, int step)
> > > step * CPPC_EM_COST_STEP;
> > > }
> > > -static int cppc_get_cpu_power(struct device *cpu_dev,
> > > +static __maybe_unused int cppc_get_cpu_power(struct device *cpu_dev,
> > > unsigned long *power, unsigned long *KHz)
> > > {
> > > unsigned long perf_step, perf_prev, perf, perf_check;
> > > @@ -547,8 +547,8 @@ static int cppc_get_cpu_power(struct device *cpu_dev,
> > > return 0;
> > > }
> > > -static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz,
> > > - unsigned long *cost)
> > > +static __maybe_unused int cppc_get_cpu_cost(struct device *cpu_dev,
> > > + unsigned long KHz, unsigned long *cost)
> > > {
> > > unsigned long perf_step, perf_prev;
> > > struct cppc_perf_caps *perf_caps;
> >
> > Should we actually run cppc_cpufreq_register_em() for
> > !CONFIG_ENERGY_MODEL ? Why?
> >
>
> Hello Viresh,
> It seems that when CONFIG_ENERGY_MODEL=n, the compiler is already
> considering cppc_cpufreq_register_em() as an empty function.
>
> Indeed, CONFIG_ENERGY_MODEL=n makes em_dev_register_perf_domain()
> an empty function, so cppc_cpufreq_register_em() is only made of
> variable definitions. This compiler optimization also explains
> why cppc_get_cpu_power() and cppc_get_cpu_cost() trigger the
> -Wunused-function warning.
>
> Putting cppc_cpufreq_register_em() inside an
> #ifdef CONFIG_ENERGY_MODEL
> guard seems also valid to me. To avoid too many empty definitions
> of cppc_cpufreq_register_em(), I guess it should be inside an
> #if defined(CONFIG_ARM64) && defined(CONFIG_ENERGY_MODEL)
> guard instead.
> Please let me know what you prefer.
In that case we shouldn't do:
cppc_cpufreq_driver.register_em = cppc_cpufreq_register_em;
as well, as that is extra work for the cpufreq core, which won't be
used at all.
So instead of __maybe_unused, lets put all dependent stuff within
CONFIG_ENERGY_MODEL ?
--
viresh
On 30-05-22, 11:42, Pierre Gondois wrote:
> Ok yes. Just to be sure and except if disagreed, I will use the
> following structure:
> #if CONFIG_ARM64
> #else
> #endif
>
> #if defined(CONFIG_ARM64) && defined(CONFIG_ENERGY_MODEL)
> int populate_efficiency_class();
> #else
> int populate_efficiency_class();
> #endif
>
> to avoid having multiple empty definitions of
> populate_efficiency_class() (for eg.) that we would have with:
> #if CONFIG_ARM64
> #if CONFIG_ENERGY_MODEL
> int populate_efficiency_class();
> #else // CONFIG_ENERGY_MODEL
> int populate_efficiency_class();
> #endif // CONFIG_ENERGY_MODEL
> #else // CONFIG_ARM64
> int populate_efficiency_class();
> #endif // CONFIG_ARM64
Look good.
--
viresh