On Mon, Nov 29 2021 at 13:20, zhenwei pi wrote: > Wrapper function x86_get_freq to get freq on a x86 platform, hide > detailed implementation for proc routine. Please rename this to x86_get_cpufreq_khz() simply because x86_get_freq() is way too unspecific. Please mention function names with 'function()' which makes it obvious what it is. Here and in the Subject. Also spell out frequency all over the place in the change log. There is no reason for using abbreviations in text. > + > +unsigned int x86_get_freq(unsigned int cpu) > +{ > + unsigned int freq = 0; > + > + if (cpu_has(&cpu_data(cpu), X86_FEATURE_TSC)) { Please use cpu_feature_enabled(X86....) and make this condition negated: if (!cpu_feature_enabled(X86_FEATURE_TSC)) return 0; which spares an indentation level and the 0 initialization of the variable. Thanks, tglx