From 08677429011a186b83ed58c3eefe153932c89323 Mon Sep 17 00:00:00 2001 From: Marcus Kida Date: Mon, 6 Jul 2026 11:14:06 +0200 Subject: [PATCH] Fix DMR hang: call AMBE codec via C function pointers, not inline asm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The radio hard-froze (needs battery pull) the instant it processed an incoming DMR frame — repeater reply, or noise on a DMR channel. FM and DMR TX-without-a- listening-repeater were fine. Cause: codec_interface.c invoked the AMBE codec through hand-written inline assembler with no operands and no clobber list, branching (BL) to an absolute address. That code's correctness depends entirely on a specific compiler's register allocation; modern GNU toolchains (both ARM GCC 10 and 14) miscompile it, corrupting state on the first codec call. Port the official R20260504 (git ebd7100) rework: replace all five inline-asm calls (2x decode, 2x encode, 1x ECC) with C function-pointer calls. Each call reproduces the exact register/stack layout the assembler set up (AAPCS: args 1-4 -> r0-r3, args 5+ -> stack) and sets the Thumb bit on the target address. Also drop the trailing semicolons from the AMBE_* address macros in codec.h so they can be used in C expressions. Builds cleanly with the GCC 14.2 Docker toolchain (no STM32CubeIDE needed) and DMR RX/TX now works on the radio. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../application/include/dmr_codec/codec.h | 12 +- .../source/dmr_codec/codec_interface.c | 153 +++++------------- 2 files changed, 48 insertions(+), 117 deletions(-) diff --git a/MDUV380_firmware/application/include/dmr_codec/codec.h b/MDUV380_firmware/application/include/dmr_codec/codec.h index f5d9350..ec680f8 100644 --- a/MDUV380_firmware/application/include/dmr_codec/codec.h +++ b/MDUV380_firmware/application/include/dmr_codec/codec.h @@ -48,13 +48,13 @@ #define QU(X) QUAUX(X) #if defined(PLATFORM_MD9600) || defined(PLATFORM_MDUV380) || defined(PLATFORM_MD380) || defined(PLATFORM_RT84_DM1701) -#define AMBE_DECODE 0x08075954; -#define AMBE_ENCODE 0x080754ac; -#define AMBE_ENCODE_ECC 0x08075864; +#define AMBE_DECODE 0x08075954 +#define AMBE_ENCODE 0x080754ac +#define AMBE_ENCODE_ECC 0x08075864 #else -#define AMBE_DECODE 0x0005543D; -#define AMBE_ENCODE 0x00054F94; -#define AMBE_ENCODE_ECC 0x0005534C; +#define AMBE_DECODE 0x0005543D +#define AMBE_ENCODE 0x00054F94 +#define AMBE_ENCODE_ECC 0x0005534C #endif extern uint8_t ambebuffer_decode[CODEC_DECODE_CONFIG_DATA_LENGTH]; extern uint8_t ambebuffer_encode[CODEC_ENCODE_CONFIG_DATA_LENGTH]; diff --git a/MDUV380_firmware/application/source/dmr_codec/codec_interface.c b/MDUV380_firmware/application/source/dmr_codec/codec_interface.c index cd70501..cc55b71 100644 --- a/MDUV380_firmware/application/source/dmr_codec/codec_interface.c +++ b/MDUV380_firmware/application/source/dmr_codec/codec_interface.c @@ -28,139 +28,70 @@ #include "dmr_codec/codec.h" #include "functions/voicePrompts.h" +#include #include static uint16_t bitbuffer_encode[72]; +/* + * The AMBE codec entry points live inside the proprietary codec blob that the + * firmware loader patches into flash at 0x0807537C. They are Thumb functions, + * so a call through a pointer requires bit 0 (the Thumb bit) to be set. + * + * These used to be called with hand-written inline assembler that did + * "BL ". Newer GNU toolchains no longer accept a direct + * branch to an absolute address (it fails to link), and even where it links the + * bare asm (no operands / no clobber list) miscompiles and hard-faults on the + * DMR path. Following the official R20260504 rework, the codec is now invoked + * through C function pointers instead. + * + * The parameter lists below reproduce, exactly, the register/stack state the + * original assembler set up (AAPCS: args 1-4 -> r0-r3, args 5+ -> stack). + */ +typedef void (*ambeDecodeFn_t)(void *waveOut, int n, void *bitBufIn, int a4, int a5, int halfFrame, void *ambeState); +typedef void (*ambeEncodeFn_t)(void *bitBufOut, int a2, void *waveIn, int n, int a5, int halfFrame, int a7, void *ambeState); +typedef void (*ambeEncodeEccFn_t)(void *bitBuf0, void *bitBuf1, int a3, void *eccState); + +#define AMBE_DECODE_FN ((ambeDecodeFn_t)(uintptr_t)(AMBE_DECODE | 1u)) +#define AMBE_ENCODE_FN ((ambeEncodeFn_t)(uintptr_t)(AMBE_ENCODE | 1u)) +#define AMBE_ENCODE_ECC_FN ((ambeEncodeEccFn_t)(uintptr_t)(AMBE_ENCODE_ECC | 1u)) + void codecDecode(uint8_t *indata_ptr, int numbBlocks) { uint16_t bitbuffer_decode[49]; - - register int r0 asm ("r0") __attribute__((unused)); - register int r1 asm ("r1") __attribute__((unused)); - register int r2 asm ("r2") __attribute__((unused)); - - for (int idx = 0; idx < numbBlocks; idx++) - { + for (int idx = 0; idx < numbBlocks; idx++) + { initFrame(indata_ptr, bitbuffer_decode); indata_ptr += 9; - soundSetupBuffer();// this just sets currentWaveBuffer but the compiler seems to optimise out the code if I try to do it in this file - r2 = (int)bitbuffer_decode; - r0 = (int)currentWaveBuffer; - r1 = (int)ambebuffer_decode; - - asm volatile ( - "PUSH {R4-R11}\n" - "SUB SP, SP, #0x10\n" - "STR R1, [SP, #0x08]\n" - "LDR R1, =0\n" - "STR R1, [SP, #0x04]\n" - "LDR R1, =0\n" - "STR R1, [SP, #0x00]\n" - "LDR R3, =0\n" - "LDR R1, =80\n" - "BL " QU(AMBE_DECODE) - "ADD SP, SP, #0x10\n" - "POP {R4-R11}" - ); - + // First half-frame + soundSetupBuffer();// sets currentWaveBuffer (the decoded-audio output buffer) + AMBE_DECODE_FN((void *)currentWaveBuffer, 80, (void *)bitbuffer_decode, 0, 0, 0, (void *)ambebuffer_decode); soundStoreBuffer(); - soundSetupBuffer();// this just sets currentWaveBuffer but the compiler seems to optimise out the code if I try to do it in this file - r2 = (int)bitbuffer_decode; - r0 = (int)currentWaveBuffer; - r1 = (int)ambebuffer_decode; - - asm volatile ( - "PUSH {R4-R11}\n" - "SUB SP, SP, #0x10\n" - "STR R1, [SP, #0x08]\n" - "LDR R1, =1\n" - "STR R1, [SP, #0x04]\n" - "LDR R1, =0\n" - "STR R1, [SP, #0x00]\n" - "LDR R3, =0\n" - "LDR R1, =80\n" - "BL " QU(AMBE_DECODE) - "ADD SP, SP, #0x10\n" - "POP {R4-R11}" - ); - + // Second half-frame + soundSetupBuffer(); + AMBE_DECODE_FN((void *)currentWaveBuffer, 80, (void *)bitbuffer_decode, 0, 0, 1, (void *)ambebuffer_decode); soundStoreBuffer(); - } + } } void codecEncodeBlock(uint8_t *outdata_ptr) { - register int r0 asm ("r0") __attribute__((unused)); - register int r1 asm ("r1") __attribute__((unused)); - register int r2 asm ("r2") __attribute__((unused)); - memset((uint8_t *)outdata_ptr, 0, 9);// fills with zeros - memset(bitbuffer_encode, 0, sizeof(bitbuffer_encode));// faster to call memset as it will be compiled as optimised code + memset(bitbuffer_encode, 0, sizeof(bitbuffer_encode)); + // First half-frame + soundRetrieveBuffer();// sets currentWaveBuffer (the audio input buffer) + AMBE_ENCODE_FN((void *)bitbuffer_encode, 0, (void *)currentWaveBuffer, 80, 0x1840, 0, 0x2000, (void *)ambebuffer_encode); - soundRetrieveBuffer();// gets currentWaveBuffer pointer used as input r2 to the encoder + // Second half-frame + soundRetrieveBuffer(); + AMBE_ENCODE_FN((void *)bitbuffer_encode, 0, (void *)currentWaveBuffer, 80, 0x0800, 1, 0x2000, (void *)ambebuffer_encode); - r0 = (int)bitbuffer_encode; - r2 = (int)currentWaveBuffer;//tmp_wavbuffer; - r1 = (int)ambebuffer_encode;// seems to be a hard coded (defined) memory address of 0x1FFF6B60. I'm not sure why it has to be hard coded, since its passed as a paramater (register) - - asm volatile ( - "PUSH {R4-R11}\n" - "SUB SP, SP, #0x14\n" - "STR R1, [SP, #0x0C]\n" - "LDR R1, =0x00002000\n" - "STR R1, [SP, #0x08]\n" - "LDR R1, =0\n" - "STR R1, [SP, #0x04]\n" - "LDR R1, =0x00001840\n" - "STR R1, [SP, #0x00]\n" - "LDR R3, =80\n" - "LDR R1, =0\n" - "BL " QU(AMBE_ENCODE) - "ADD SP, SP, #0x14\n" - "POP {R4-R11}" - ); - - soundRetrieveBuffer();// gets currentWaveBuffer pointer used as input r2 to the encoder - - r0 = (int)bitbuffer_encode; - r2 = (int)currentWaveBuffer;//tmp_wavbuffer; - r1 = (int)ambebuffer_encode; - - asm volatile ( - "PUSH {R4-R11}\n" - "SUB SP, SP, #0x14\n" - "STR R1, [SP, #0x0C]\n" - "LDR R1, =0x00002000\n" - "STR R1, [SP, #0x08]\n" - "LDR R1, =1\n" - "STR R1, [SP, #0x04]\n" - "LDR R1, =0x00000800\n" - "STR R1, [SP, #0x00]\n" - "LDR R3, =80\n" - "LDR R1, =0\n" - "BL " QU(AMBE_ENCODE) - "ADD SP, SP, #0x14\n" - "POP {R4-R11}" - ); - - r0 = (int)bitbuffer_encode; - r1 = (int)ambebuffer_encode_ecc; - - asm volatile ( - "PUSH {R4-R11}\n" - "SUB SP, SP, #0x14\n" - "MOV R3, R1\n" - "LDR R2, =0\n" - "MOV R1, R0\n" - "BL " QU(AMBE_ENCODE_ECC) - "ADD SP, SP, #0x14\n" - "POP {R4-R11}" - ); + // ECC pass + AMBE_ENCODE_ECC_FN((void *)bitbuffer_encode, (void *)bitbuffer_encode, 0, (void *)ambebuffer_encode_ecc); for (int i = 0; i < 72; i++) {