On Friday, June 16, 2017 09:49:00 PM Len Brown wrote: > On Fri, Jun 16, 2017 at 8:30 PM, Rafael J. Wysocki <r...@rjwysocki.net> wrote: > > On Wednesday, June 07, 2017 07:39:13 PM Len Brown wrote: > >> From: Len Brown <len.br...@intel.com>
[cut] > > > > I wonder if we could change intel_pstate_get() to simply return > > aperfmperf_khz_on_cpu(cpu_num)? > > > > That would allow us to avoid the extra branch here and get rid of the > > #ifdef x86 from the header. > > The reason I put the hook here is specifically so that the same > code would always be called on the x86 architecture, > no not matter what cpufreq driver is loaded. > > Yes, alternatively, all possible driver.get routines could be updated > to call the same routine. That is acpi-cpufreq.c, intel_pstate.c, > others? Just acpi-cpufreq.c and intel_pstate.c. Moreover, I wouldn't change the behavior on systems using acpi-cpufreq.c, because why really? And some users may complain that now what they see in cpuinfo_cur_freq is different from what they saw there before. That leaves us with just intel_pstate. In addition to that, it would be good to avoid reading APERF and MPERF in the cases when we already have the values. I have an idea on how to do that, but I'll just need to prepare a patch. > Do I think that saving a branch is meaningful in a system call to retrieve > the average frequency from the kernel? No, I don't. > > I'm not religious about "if (CONFIG_X86)" appearing in cpufreq.h. > If somebody really cares, then the purist approach would be > to add an additional arch-specific-hook header -- though that seemed > overkill to me... I'm not a fan of arch stuff in general code, C or headers. If it is necessary to add an arch header, let's add it, but at this point I'm not sure about that even. Thanks, Rafael