On Fri, 9 Sep 2016, Harry Pan wrote: > - if (apply_quirk) > + if (apply_quirk == RAPL_HSX_QUIRK) > rapl_hw_unit[RAPL_IDX_RAM_NRG_STAT] = 16; > > /* > + * Some Atom processors (BYT/BSW) have 2^ESU microjoules increment, > + * refer to Software Developers' Manual, Vol. 3C, Order No. 325384, > + * Table 35-8 of MSR_RAPL_POWER_UNIT > + */ > + if (apply_quirk == RAPL_BYT_QUIRK) { > + for (i = 0; i < NR_RAPL_DOMAINS; i++) > + rapl_hw_unit[i] = 32 - rapl_hw_unit[i]; > + }
switch(quirk) if at all, but see below. > + /* > * Calculate the timer rate: > * Use reference of 200W for scaling the timeout to avoid counter > * overflows. 200W = 200 Joules/sec > @@ -702,47 +742,53 @@ static int __init init_rapl_pmus(void) > { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&init } > > struct intel_rapl_init_fun { > - bool apply_quirk; > + enum rapl_quirk apply_quirk; This is silly. Make apply_quirk a function pointer and provide functions for the different quirks. > int cntr_mask; > struct attribute **attrs; > }; > > static const struct intel_rapl_init_fun snb_rapl_init __initconst = { > - .apply_quirk = false, > + .apply_quirk = RAPL_NO_QUIRK, Zero ininitalization has no real value other than consuming state space. Thanks, tglx