[PATCH] Revert "time: add weak annotation to timer_read_counter declaration"

Rob Herring robh at kernel.org
Wed Jan 11 21:49:39 CET 2023


On Wed, Jan 4, 2023 at 6:09 PM Harald Seiler <hws at 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.
>
> [1]: https://stackoverflow.com/questions/31203402/gcc-behavior-for-unresolved-weak-functions
>
> 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


More information about the U-Boot mailing list