This is an automated email from the ASF dual-hosted git repository. linguini1 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 689ed188c617a124b8f53baf102d521bcc9ac438 Author: Kerogit <[email protected]> AuthorDate: Thu Mar 5 08:52:22 2026 +0100 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 <[email protected]> --- arch/Kconfig | 31 +++++++++++++++++++++++++++++++ drivers/timers/arch_alarm.c | 16 ++++++++++++++++ drivers/timers/arch_timer.c | 42 +++++++++++++++++++++++++++++++++++------- sched/clock/clock_delay.c | 43 ++++++++++++++++++++++++++++++++++++++++--- 4 files changed, 122 insertions(+), 10 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 7cd6ec4f10d..0085b50d1ad 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -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, diff --git a/drivers/timers/arch_alarm.c b/drivers/timers/arch_alarm.c index 557a326f36e..b8eb29bf8af 100644 --- a/drivers/timers/arch_alarm.c +++ b/drivers/timers/arch_alarm.c @@ -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 * diff --git a/drivers/timers/arch_timer.c b/drivers/timers/arch_timer.c index c0cb09fd9d6..5e3b0f988cd 100644 --- a/drivers/timers/arch_timer.c +++ b/drivers/timers/arch_timer.c @@ -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 * diff --git a/sched/clock/clock_delay.c b/sched/clock/clock_delay.c index cf3e2f0c902..f217477505a 100644 --- a/sched/clock/clock_delay.c +++ b/sched/clock/clock_delay.c @@ -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 *
