Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33797 )
Change subject: arch/riscv: Fix arch timer timebase ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/33797/1/src/arch/riscv/arch_timer.c File src/arch/riscv/arch_timer.c:
https://review.coreboot.org/#/c/33797/1/src/arch/riscv/arch_timer.c@31 PS1, Line 31: die("time not set in HLS");
does this HAVE to be a die? Can we somehow stumble along with bad values?
Well 'die()' is just another 'printk()' called before 'console_init()' here that would not show up, even on UART TX line.
With 'printk()' called after 'console_init()', and if UART code hits 'udelay()', we have infinite recursion here, possibly a silent reset loop is what you will see.
It's possibly me who put 'udelay()' there in the UART_MEM code, ideas are welcome on how to get it removed while still having some controlled timeouts there. Timeouts don't have to be accurate but reasonable.
https://review.coreboot.org/#/c/33797/1/src/arch/riscv/arch_timer.c@36 PS1, Line 36: mono_time_set_usecs(mt, (long)(raw / clocks_per_usec)); unsigned long ?
There is a fair amount of loss in accuracy should you want to support timebases that are not multiples of 1 MHz. HW timer running at 1.999 MHz would be evil. Consider leaving a comment about that or improve the math.