On Wed, Jan 4, 2023 at 6:09 PM Harald Seiler hws@denx.de wrote:
This reverts commit 65ba7add0d609bbd035b8d42fafdaf428ac24751.
A weak extern is a nasty sight to behold: If the symbol is never defined, on ARM, the linker will replace the function call with a NOP. This behavior isn't well documented but there are at least some hints to it [1].
When timer_read_counter() is not defined, this obviously does the wrong thing here and it does so silently. The consequence is that a board without timer_read_counter() will sleep for random amounts and generally have erratic get_ticks() values.
Drop the __weak annotation of the extern so a linker error is raised when timer_read_counter() is not defined. This is okay, the original reason for the reverted change - breaking the sandbox build - no longer applies.
Final sidenote: This was the only weak extern in the entire tree at this time as far as I can tell. I guess we should avoid introduction of them again as they are obviously a very big footgun.
Fixes: 65ba7add0d60 ("time: add weak annotation to timer_read_counter declaration")
I don't agree that this is a fix to the above commit. Applying it at any point after commit 65ba7add0d60 would not work in all cases. It may not be needed any more, but that is probably due to the addition of the timer class 2 years later, not that the commit was wrong/broken at the time.
Rob