sched/clock/clock_delay: added config flag to remove weak up_udelay

While attempting to create architecture-specific implementation
of up_udelay, it was discovered that the overriding function is not
included in the final binary, the weak implementation was used instead.

Further investigation and experimentation showed that the linker
only overrides the weak implementation with the custom one if
the custom one is present in a .c source file that contains at least
one other function that is called from somewhere. Some additional
testing revealed that at least one other already present up_udelay
override (rv32m1-vega:nsh) is affected by this.

In a short mailing list discussion it was determined that this
is a likely result of using static libraries during the build process
and it was suggested to introduce configuration option that will
exclude weak implementations of the function from the build altogether.
This patch does that.

This patch does not enable this configuration option for any existing
board/chip because doing so would change its behaviour and needs
to be tested by users of the hardware.

Also changed is the static assertion in sched/clock/clock_delay.c
to not prevent building the code when architecture declares that
it does not use BOARD_LOOPSPERMSEC to determine required loop count.
BOARD_LOOPSPERMSEC is made undefined in such case.

Patch was tested by building breadxavr:nsh (identical binary by SHA256),
rv32m1-vega:nsh (identical text section) and rv-virt:nsh (text section
differs because of different ordering of functions in the binary, ostest
passed though.)

Signed-off-by: Kerogit <kr.git@kerogit.eu>
This commit is contained in:
Kerogit 2026-03-05 08:52:22 +01:00 committed by Matteo Golin
parent a7352902ae
commit 689ed188c6
4 changed files with 122 additions and 10 deletions

View file

@ -560,6 +560,36 @@ config ARCH_LDST_16BIT_NOT_ATOMIC
(The same applies for non-interrupt context write and interrupt
context read.)
config ARCH_HAVE_UDELAY
bool
default n
---help---
This configuration option is set for architectures that define
their own version of up_udelay. That function is defined as weak
but due to linker behaviour with regards to static libraries,
it was found that the overriding definition is not included
in the resulting binary, the default (weak) one is still used.
(Found with GCC 14.2 on AVR and Risc-V but likely present
elsewhere too. Manifests when up_udelay override is the only
function in the source file.)
If set, the default up_udelay function is excluded from build.
config ARCH_HAVE_DYNAMIC_UDELAY
bool
default n
depends on ARCH_HAVE_UDELAY
---help---
This configuration option is set if architecture-specific up_udelay
function does not use CONFIG_BOARD_LOOPSPERMSEC. This should be used
for architectures/boards where the user has the option to choose
clock frequency of the CPU.
(Keep in mind that up_udelay function may also be used in situations
after a failure - the implementation should always re-read current
hardware settings as anything stored in memory may be corrupted.)
config ARCH_HAVE_TESTSET
bool
default n
@ -1366,6 +1396,7 @@ comment "Board Settings"
config BOARD_LOOPSPERMSEC
int "Delay loops per millisecond"
depends on !ARCH_HAVE_DYNAMIC_UDELAY
default -1
---help---
Simple delay loops are used by some logic, especially during boot-up,

View file

@ -39,8 +39,16 @@
/* If no value is given, we proceed with 0 since a one-shot timer is used for
* accurate delays. A runtime DEBUGASSERT catches the case where the one-shot
* timer lower-half isn't registered in time.
*
* If ARCH_HAVE_DYNAMIC_UDELAY is set, BOARD_LOOPSPERMSEC is unset.
* Considering the above, it should not be used. Set a default value of -1,
* turning this case into an already handled one.
*/
#ifndef CONFIG_BOARD_LOOPSPERMSEC
# define CONFIG_BOARD_LOOPSPERMSEC -1
#endif
#if CONFIG_BOARD_LOOPSPERMSEC == -1
# undef CONFIG_BOARD_LOOPSPERMSEC
# define CONFIG_BOARD_LOOPSPERMSEC 0
@ -181,13 +189,21 @@ void weak_function up_mdelay(unsigned int milliseconds)
* Delay inline for the requested number of microseconds.
* WARNING: NOT multi-tasking friendly
*
* This function is both compiled optionally based on ARCH_HAVE_UDELAY
* and declared with weak attribute. See comment of up_udelay
* implementation in sched/clock/clock_delay.c for explanation.
*
****************************************************************************/
#ifndef CONFIG_ARCH_HAVE_UDELAY
void weak_function up_udelay(useconds_t microseconds)
{
up_ndelay(NSEC_PER_USEC * microseconds);
}
#endif
/****************************************************************************
* Name: up_ndelay
*

View file

@ -39,16 +39,22 @@
/* If no value is given, we proceed with 0 since a timer is used for accurate
* delays. A runtime DEBUGASSERT catches the case where the timer lower-half
* isn't registered in time.
*
* Value is unset if ARCH_HAVE_DYNAMIC_UDELAY is set. In that case,
* ARCH_HAVE_UDELAY is also set and the only user of these values
* (udelay_coarse) is excluded from the build.
*/
#if CONFIG_BOARD_LOOPSPERMSEC == -1
# undef CONFIG_BOARD_LOOPSPERMSEC
# define CONFIG_BOARD_LOOPSPERMSEC 0
#endif
#ifndef CONFIG_ARCH_HAVE_UDELAY
# if CONFIG_BOARD_LOOPSPERMSEC == -1
# undef CONFIG_BOARD_LOOPSPERMSEC
# define CONFIG_BOARD_LOOPSPERMSEC 0
# endif
#define CONFIG_BOARD_LOOPSPER100USEC ((CONFIG_BOARD_LOOPSPERMSEC+5)/10)
#define CONFIG_BOARD_LOOPSPER10USEC ((CONFIG_BOARD_LOOPSPERMSEC+50)/100)
#define CONFIG_BOARD_LOOPSPERUSEC ((CONFIG_BOARD_LOOPSPERMSEC+500)/1000)
# define CONFIG_BOARD_LOOPSPER100USEC ((CONFIG_BOARD_LOOPSPERMSEC+5)/10)
# define CONFIG_BOARD_LOOPSPER10USEC ((CONFIG_BOARD_LOOPSPERMSEC+50)/100)
# define CONFIG_BOARD_LOOPSPERUSEC ((CONFIG_BOARD_LOOPSPERMSEC+500)/1000)
#endif
/****************************************************************************
* Private Types
@ -127,6 +133,18 @@ static void udelay_accurate(useconds_t microseconds)
}
}
#ifndef CONFIG_ARCH_HAVE_UDELAY
/****************************************************************************
* Name: udelay_coarse
*
* Description:
* Wait loop called (only) by up_udelay if udelay_accurate
* is not available. (Excluded from the build if up_udelay is also
* excluded from the build.)
*
****************************************************************************/
static void udelay_coarse(useconds_t microseconds)
{
volatile int i;
@ -176,6 +194,8 @@ static void udelay_coarse(useconds_t microseconds)
}
}
#endif /* ifndef CONFIG_ARCH_HAVE_UDELAY */
static bool timer_callback(FAR uint32_t *next_interval, FAR void *arg)
{
#ifdef CONFIG_SCHED_TICKLESS
@ -464,8 +484,14 @@ void weak_function up_mdelay(unsigned int milliseconds)
*
* *** NOT multi-tasking friendly ***
*
* This function is both compiled optionally based on ARCH_HAVE_UDELAY
* and declared with weak attribute. See comment of up_udelay
* implementation in sched/clock/clock_delay.c for explanation.
*
****************************************************************************/
#ifndef CONFIG_ARCH_HAVE_UDELAY
void weak_function up_udelay(useconds_t microseconds)
{
if (g_timer.lower != NULL)
@ -478,6 +504,8 @@ void weak_function up_udelay(useconds_t microseconds)
}
}
#endif
/****************************************************************************
* Name: up_ndelay
*

View file

@ -67,17 +67,27 @@
"new value to apache/nuttx."
#endif
#ifndef CONFIG_ARCH_HAVE_DYNAMIC_UDELAY
static_assert(CONFIG_BOARD_LOOPSPERMSEC != -1,
"Configure BOARD_LOOPSPERMSEC to non-default value.");
#define CONFIG_BOARD_LOOPSPER100USEC ((CONFIG_BOARD_LOOPSPERMSEC+5)/10)
#define CONFIG_BOARD_LOOPSPER10USEC ((CONFIG_BOARD_LOOPSPERMSEC+50)/100)
#define CONFIG_BOARD_LOOPSPERUSEC ((CONFIG_BOARD_LOOPSPERMSEC+500)/1000)
/* If ARCH_HAVE_DYNAMIC_UDELAY is set, BOARD_LOOPSPERMSEC is unset,
* calculate these optionally. (Only used in udelay_coarse and
* that function is excluded from the build if these end up undefined.)
*/
# define CONFIG_BOARD_LOOPSPER100USEC ((CONFIG_BOARD_LOOPSPERMSEC+5)/10)
# define CONFIG_BOARD_LOOPSPER10USEC ((CONFIG_BOARD_LOOPSPERMSEC+50)/100)
# define CONFIG_BOARD_LOOPSPERUSEC ((CONFIG_BOARD_LOOPSPERMSEC+500)/1000)
#endif
/****************************************************************************
* Private Functions
****************************************************************************/
#ifndef CONFIG_ARCH_HAVE_UDELAY
static void udelay_coarse(useconds_t microseconds)
{
volatile int i;
@ -125,6 +135,8 @@ static void udelay_coarse(useconds_t microseconds)
}
}
#endif
/****************************************************************************
* Public Functions
****************************************************************************/
@ -151,13 +163,38 @@ void weak_function up_mdelay(unsigned int milliseconds)
*
* *** NOT multi-tasking friendly ***
*
* Note that this method is both marked weak and compiled optionally
* based on ARCH_HAVE_UDELAY option. This is redundant but kept
* as a temporary measure.
*
* It was discovered that sometimes the weak attribute is not enough
* to have the method replaced with other, non-weak implementation.
* (Described in more detail in help text for the ARCH_HAVE_UDELAY
* Kconfig option.) The ARCH_HAVE_UDELAY option is meant to remedy
* this but the author of the change does not own all the hardware
* that implements its own up_udelay and is affected by this problem.
*
* Enabling ARCH_HAVE_UDELAY for every such chip is therefore impossible,
* the change would be untested. Instead, the weak attribute is kept,
* preserving previous behaviour of current code. If, at some future
* time, the change is tested for other chips that implement up_udelay,
* the weak attribute and this comment can be removed.
*
* The same also applies to up_udelay implementations
* in drivers/timers/arch_alarm.c and drivers/timers/arch_timer.c
* and this comment is referenced there.
*
****************************************************************************/
#ifndef CONFIG_ARCH_HAVE_UDELAY
void weak_function up_udelay(useconds_t microseconds)
{
udelay_coarse(microseconds);
}
#endif
/****************************************************************************
* Name: up_ndelay
*