Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35148 )
Change subject: soc/intel/common/timer: Make TSC frequency calculation dynamically ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35148/4/src/soc/intel/common/block/... File src/soc/intel/common/block/timer/timer.c:
https://review.coreboot.org/c/coreboot/+/35148/4/src/soc/intel/common/block/... PS4, Line 24: #define CPU_MODEL_INTEL_ATOM_DENVERTON 0x5F
Why remove the header that provided this?
as we are moving for hard coding to dynamic detection and going forward seeing soc implementing cpuid.15h to support nominal TSC freq, we don't need to hardcoded model number. DNV might be only case which doesn't support CPUID.15h
https://review.coreboot.org/c/coreboot/+/35148/4/src/soc/intel/common/block/... PS4, Line 67: assert(cpuidr_15h.ebx != 0 || cpuidr_15h.eax != 0);
I know this is what I wrote, but I have second thoughts about the assert() now. […]
my bad, i shouldn't have added assert() uart won't be up by this time and it would cause hang. i could see Linux upstream is also returning 0 like i used to do in patchset 3
https://review.coreboot.org/c/coreboot/+/35148/4/src/soc/intel/common/block/... PS4, Line 78: core_crystal_nominal_freq_khz = 25000;
Indentation is confusing. […]
make sense, will do something like that.
https://review.coreboot.org/c/coreboot/+/35148/4/src/soc/intel/common/block/... PS4, Line 94: return (core_crystal_nominal_freq_khz * cpuidr_15h.ebx /
Returning 0 here is what I want to avoid. […]
please try to avoid dependency here with your patch (I have a topic thread 'monotonic-timer'), please feel free to change code after proper testing.
Also refer to 5.23 kernel code, this is meaningful and i'm porting the same here..
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch...