Messages in this thread | | | Date | Mon, 30 May 2022 11:42:49 +0200 | Subject | Re: [PATCH v1] cpufreq: CPPC: Fix unused-function warning | From | Pierre Gondois <> |
| |
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 <zhangshaokun@hisilicon.com> >>>> Signed-off-by: Pierre Gondois <pierre.gondois@arm.com> >>>> --- >>>> 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
| |