* Daniel Drake <[email protected]> wrote:
> On Mon, Apr 22, 2019 at 8:04 PM Ingo Molnar <[email protected]> wrote: > > Minor style nit: the parentheses are unnecessary, integer expressions > > like this are evaluated left to right and multiplication and division has > > the same precedence. > > Fair point, although the same could be said for cpu_khz_from_msr(). Yes, this standardization on mhz, if it makes sense, might have to be propagated a bit to function names and any other variables. Another naming quirk: what unit is 'lapic_timer_frequency' in? AFAICS it's a "period" unit (number of clock cycles per jiffy), not a frequency (which is number of cycles per second) - so the better name might be lapic_timer_period? > > But it might also make sense to actually store crystal_mhz instead of > > crystal_khz, because both CPUID 15H and 16H provides MHz values. > > > > That way the above expression would simplify to: > > > > lapic_timer_frequency = crystal_mhz / HZ; > > Wouldn't it be > lapic_timer_frequency = crystal_mhz * 1000000 / HZ; > ? Sorry, what I wanted to suggest is crystal_hz, not crystal_mhz. I.e. store the raw unit that comes out of the CPUID, which appears to be HZ, right? That would change the calculation to: lapic_timer_period = crystal_hz / HZ; Note how clearly it's visible in that calculation that what we calculate there is a 'period', not a frequency. Thanks, Ingo
