From 12b2a02f5b79163a28f11458b70668bc65d13689 Mon Sep 17 00:00:00 2001 From: mikecarper Date: Fri, 23 Oct 2026 02:19:44 -0700 Subject: [PATCH] Guard RAK3401 SPI clock experiments and enable nRF SPIM3 DMA workaround --- platformio.ini | 2 + test/test_rak3401_buffered_radio_hal.py | 49 ++++++++++++++++++++++++- test/test_rak3401_onboard_radio.py | 2 +- variants/rak3401/target.cpp | 24 ++++++++++-- 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/platformio.ini b/platformio.ini index daed06d25..332734396 100644 --- a/platformio.ini +++ b/platformio.ini @@ -209,6 +209,8 @@ build_flags = ${arduino_base.build_flags} ; PendSV reaches this FreeRTOS function from assembly, outside the LTO graph. -Wl,--undefined=vTaskSwitchContext -D NRF52_PLATFORM + ; Keep SPIM3 EasyDMA TX correct when CPU/IRQs access the same RAM bank. + -D NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED=1 -D ED25519_USE_CRYPTO_SHA512=1 ; share the SHA-512 used by signature verification -D LFS_NO_ASSERT=1 -D EXTRAFS=1 diff --git a/test/test_rak3401_buffered_radio_hal.py b/test/test_rak3401_buffered_radio_hal.py index 66d54fb2b..d85b2a80d 100644 --- a/test/test_rak3401_buffered_radio_hal.py +++ b/test/test_rak3401_buffered_radio_hal.py @@ -53,6 +53,7 @@ HARNESS = r''' #include #include #include +@CLOCK_POLICY@ SPIClass SPI, SPI1; constexpr unsigned P_LORA_NSS=1, P_LORA_DIO_1=2, P_LORA_RESET=3, P_LORA_BUSY=4; struct Module { @@ -60,7 +61,7 @@ struct Module { Module(ArduinoHal* adapter, unsigned cs, unsigned irq, unsigned rst, unsigned busy) : hal(adapter) { assert(cs==1 && irq==2 && rst==3 && busy==4); - assert(hal->spi == &SPI1 && hal->spiSettings.hz == 8000000); + assert(hal->spi == &SPI1 && hal->spiSettings.hz == MC_RAK3401_SPI_HZ); } }; struct Radio { @@ -99,6 +100,8 @@ int main() { class Rak3401BufferedRadioHalTests(unittest.TestCase): def test_actual_target_uses_one_transfer_per_command_on_same_spi1_at_8mhz(self): target = (ROOT / "variants/rak3401/target.cpp").read_text(encoding="ascii") + clock_policy = target[target.index("#ifndef MC_RAK3401_SPI_HZ"): + target.index("\nnamespace {")] construction = "\n".join(re.findall( r"^(?:static Nrf52BufferedRadioHal radioHal\([^;]+;|" r"RADIO_CLASS radio = new Module\([^;]+;)$", target, re.MULTILINE)) @@ -117,7 +120,8 @@ class Rak3401BufferedRadioHalTests(unittest.TestCase): (base / "helpers/radiolib").mkdir(parents=True) (base / "hal/Arduino/ArduinoHal.h").write_text(MOCK_HAL, encoding="ascii") cpp, executable = base / "test.cpp", base / "test" - cpp.write_text(HARNESS.replace("@CONSTRUCTION@", construction), encoding="ascii") + cpp.write_text(HARNESS.replace("@CONSTRUCTION@", construction) + .replace("@CLOCK_POLICY@", clock_policy), encoding="ascii") for name, implementation, succeeds in (("buffered", helper, True), ("per-byte", legacy, False)): with self.subTest(implementation=name): @@ -135,6 +139,47 @@ class Rak3401BufferedRadioHalTests(unittest.TestCase): self.assertNotEqual(ran.returncode, 0, "per-byte negative control did not fail") + def test_clock_dial_requires_correct_controller_and_global_dma_workaround(self): + target = (ROOT / "variants/rak3401/target.cpp").read_text(encoding="ascii") + policy = target[target.index("#ifndef MC_RAK3401_SPI_HZ"): + target.index("\nnamespace {")] + construction = "\n".join(re.findall( + r"^(?:static Nrf52BufferedRadioHal radioHal\([^;]+;|" + r"RADIO_CLASS radio = new Module\([^;]+;)$", target, re.MULTILINE)) + fixture = HARNESS.replace("@CLOCK_POLICY@", policy).replace("@CONSTRUCTION@", construction) + with tempfile.TemporaryDirectory() as directory: + folder = Path(directory) + (folder / "hal/Arduino").mkdir(parents=True) + (folder / "hal/Arduino/ArduinoHal.h").write_text(MOCK_HAL, encoding="ascii") + cpp, binary = folder / "test.cpp", folder / "test.exe" + cpp.write_text(fixture, encoding="ascii") + cases = [([], True)] + [ + ([f"MC_RAK3401_SPI_HZ={clock}"], True) + for clock in (2000000, 4000000, 8000000)] + [ + (["MC_RAK3401_SPI_HZ=16000000", "SPI_32MHZ_INTERFACE=1", + "NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED=1"], True), + (["MC_RAK3401_SPI_HZ=16000000"], False), + (["MC_RAK3401_SPI_HZ=16000000", "SPI_32MHZ_INTERFACE=1"], False), + (["MC_RAK3401_SPI_HZ=16000000", "SPI_32MHZ_INTERFACE=0", + "NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED=1"], False), + (["MC_RAK3401_SPI_HZ=16000000", "SPI_32MHZ_INTERFACE=1", + "NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED=0"], False), + (["MC_RAK3401_SPI_HZ=12000000"], False), + (["MC_RAK3401_SPI_HZ=32000000"], False)] + for flags, valid in cases: + with self.subTest(flags=flags): + built = subprocess.run([os.environ.get("CXX", "g++"), "-std=c++17", + "-Wall", "-Wextra", "-Werror", *["-D" + flag for flag in flags], + "-I", str(folder), "-I", str(ROOT / "src"), str(cpp), + "-o", str(binary)], capture_output=True, text=True) + if valid: + self.assertEqual(built.returncode, 0, built.stderr) + ran = subprocess.run([str(binary)], capture_output=True, text=True) + self.assertEqual(ran.returncode, 0, ran.stderr) + else: + self.assertNotEqual(built.returncode, 0) + self.assertIn("RAK3401", built.stderr) + if __name__ == "__main__": unittest.main() diff --git a/test/test_rak3401_onboard_radio.py b/test/test_rak3401_onboard_radio.py index 498b7c022..9a8469feb 100644 --- a/test/test_rak3401_onboard_radio.py +++ b/test/test_rak3401_onboard_radio.py @@ -473,7 +473,7 @@ class Rak3401OnboardRadioTests(unittest.TestCase): "P_LORA_RESET, P_LORA_BUSY)", target) # External-radio construction and retry must retain their SPI1 owner. external_target = (VARIANT / "target.cpp").read_text() - self.assertIn("Nrf52BufferedRadioHal radioHal(SPI1, 8000000)", external_target) + self.assertIn("Nrf52BufferedRadioHal radioHal(SPI1, MC_RAK3401_SPI_HZ)", external_target) self.assertIn("new Module(&radioHal, P_LORA_NSS, P_LORA_DIO_1, " "P_LORA_RESET, P_LORA_BUSY)", external_target) external_init = extract_braced(external_target, "bool radio_init()") diff --git a/variants/rak3401/target.cpp b/variants/rak3401/target.cpp index c9453f587..cb6fe5880 100644 --- a/variants/rak3401/target.cpp +++ b/variants/rak3401/target.cpp @@ -8,6 +8,23 @@ #include #endif +#ifndef MC_RAK3401_SPI_HZ +#define MC_RAK3401_SPI_HZ 8000000 +#endif +#if MC_RAK3401_SPI_HZ != 2000000 && MC_RAK3401_SPI_HZ != 4000000 \ + && MC_RAK3401_SPI_HZ != 8000000 && MC_RAK3401_SPI_HZ != 16000000 +#error "RAK3401 radio SPI clock must be 2, 4, 8 or 16 MHz" +#endif +#if MC_RAK3401_SPI_HZ == 16000000 +#if !defined(SPI_32MHZ_INTERFACE) || SPI_32MHZ_INTERFACE != 1 +#error "RAK3401 16 MHz radio requires SPI1 on SPIM3 (SPI_32MHZ_INTERFACE=1)" +#endif +#if !defined(NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED) \ + || NRFX_SPIM3_NRF52840_ANOMALY_198_WORKAROUND_ENABLED != 1 +#error "RAK3401 16 MHz radio requires the global nrfx SPIM3 anomaly 198 workaround" +#endif +#endif + namespace { // This is the optional private radio bus, NOT the external RAK13302's SPI1. @@ -126,9 +143,10 @@ RAK3401Board board; // controller instance for startup and watchdog recovery; resetting the radio // must not replace or tear down the SPI object used by its Module. // Batch each RadioLib command into one SPIM transfer rather than starting a -// transfer per byte. Use the SPI1-supported 8 MHz clock and retain RadioLib's BUSY, -// chip-select, command status, and IRQ handling. -static Nrf52BufferedRadioHal radioHal(SPI1, 8000000); +// transfer per byte. Keep 8 MHz by default. A 16 MHz HIL comparison must move +// SPI1 to SPIM3 for the whole framework build and enable its DMA workaround; +// changing the speed never replaces this bus or its recovery/handoff path. +static Nrf52BufferedRadioHal radioHal(SPI1, MC_RAK3401_SPI_HZ); RADIO_CLASS radio = new Module(&radioHal, P_LORA_NSS, P_LORA_DIO_1, P_LORA_RESET, P_LORA_BUSY); WRAPPER_CLASS radio_driver(radio, board);