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.git@kerogit.eu>
This commit is contained in:
Kerogit 2025-06-05 13:03:26 +02:00 committed by Xiang Xiao
parent 13ac3d5e57
commit 25876e327e

View file

@ -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__ */