This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 25876e327e arch/avr: fix atomic load functions from arch_atomic.c
25876e327e is described below
commit 25876e327ea432d2cf23485179fbf66effa158c4
Author: Kerogit <kr....@kerogit.eu>
AuthorDate: Thu Jun 5 13:03:26 2025 +0200
arch/avr: fix atomic load functions from arch_atomic.c
For AVR, atomic functions generated by LOAD macro turn into load surrounded
by up_irq_save and up_irq_restore. The generated code was incorrect as can
be seen from disassembly of __atomic_load_4:
in r18, 0x3f ; store interrupts enabled flag
cli ; disable interrupts
out 0x3f, r18 ; restore the flag
movw r30, r24 ; copy parameter (address) to pointer register
ld r22, Z ; indirect load to return value registers
ldd r23, Z+1
ldd r24, Z+2
ldd r25, Z+3
ret ; return
The interrupts are disabled to be immediately re-enabled, the load only
takes
place after that.
Both up_irq_save and up_irq_restore are defined in inline assembly. Other
architectures (x86/486, Risc-V) mark this assembly with clobbers: memory.
Doing the same thing for AVR alleviates the problem:
in r18, 0x3f ; store interrupts enabled flag
cli ; disable interrupts
movw r30, r24 ; copy address
ld r22, Z ; load
ldd r23, Z+1
ldd r24, Z+2
ldd r25, Z+3
out 0x3f, r18 ; restore interrupts enabled flag
ret ; return
Besides compiling the code and checking the assembly, this was tested
with a custom stress application on AVR128DA28.
Assembly of up_irq_enable is marked in the same way with regards to
clobbers.
This patch also removes two functions that are not called from anywhere
(up_irq_disabled, putsreg)
Signed-off-by: Kerogit <kr....@kerogit.eu>
---
arch/avr/include/avr/irq.h | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/arch/avr/include/avr/irq.h b/arch/avr/include/avr/irq.h
index bdf164638f..53c5ab6e7a 100644
--- a/arch/avr/include/avr/irq.h
+++ b/arch/avr/include/avr/irq.h
@@ -151,11 +151,6 @@ static inline_function irqstate_t getsreg(void)
return sreg;
}
-static inline_function void putsreg(irqstate_t sreg)
-{
- asm volatile ("out __SREG__, %s" : : "r" (sreg) :);
-}
-
/* Return the current value of the stack pointer */
static inline_function uint16_t up_getsp(void)
@@ -178,12 +173,7 @@ static inline_function uint16_t up_getsp(void)
static inline_function void up_irq_enable()
{
- asm volatile ("sei" ::);
-}
-
-static inline_function void up_irq_disabled()
-{
- asm volatile ("cli" ::);
+ asm volatile ("sei" ::: "memory");
}
/* Save the current interrupt enable state & disable all interrupts */
@@ -195,7 +185,7 @@ static inline_function irqstate_t up_irq_save(void)
(
"\tin %0, __SREG__\n"
"\tcli\n"
- : "=&r" (sreg) ::
+ : "=&r" (sreg) :: "memory"
);
return sreg;
}
@@ -204,7 +194,7 @@ static inline_function irqstate_t up_irq_save(void)
static inline_function void up_irq_restore(irqstate_t flags)
{
- asm volatile ("out __SREG__, %0" : : "r" (flags) :);
+ asm volatile ("out __SREG__, %0" : : "r" (flags) : "memory");
}
#endif /* __ASSEMBLY__ */
