From 05472c14937f153c441d5b51a7e81093a7c1a181 Mon Sep 17 00:00:00 2001
From: Jason <83615043+JJassonn69@users.noreply.github.com>
Date: Mon, 4 May 2026 22:06:08 +0545
Subject: [PATCH] =?UTF-8?q?fix(mcu):=20F-4.5=20+=20F-4.6=20=E2=80=94=20AD9?=
 =?UTF-8?q?523=20heap/lifecycle=20hygiene?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

F-4.5: ad9523_setup() malloc's both an ad9523_dev and a no_os SPI
descriptor (ad9523.c:430,435). Previously the dev pointer was local
to configure_ad9523() and fell out of scope on return — every
recovery cycle (ERROR_AD9523_CLOCK → re-run configure_ad9523) leaked
one struct + one SPI desc. STM32F7 heap is bounded; sustained
brown-out flapping would eventually exhaust it. Move dev to a file-
scope `g_ad9523_dev` and call ad9523_remove() at the top of
configure_ad9523() to free the previous instance before re-setup.
Initial boot path is unaffected (g_ad9523_dev=NULL → remove call
gated by NULL check).

F-4.6: ad9523_setup() called ad9523_calibrate() but discarded its
return value (ad9523.c:707). VCO calibration can fail silently — if
the target VCO is outside the 3.6-4.0 GHz band (e.g. F-4.1 wipe left
PLL2 N=16, target 1.6 GHz), calibrate would report failure but setup
still proceeded to ad9523_status(), where PLL2_LD might pass
spuriously. Capture and propagate the calibrate return so a failed
calibration aborts setup with a clear non-zero status code instead
of being absorbed.

Both fixes are mechanical and don't change correct-path behaviour.
Regression: 86/0 (mocks bypass real driver, so F-4.6 is not covered
by tests; F-4.5 changes are in main.cpp and don't trip mocked
configure_ad9523).
---
 .../9_1_1_C_Cpp_Libraries/ad9523.c               |  8 +++++++-
 .../9_1_3_C_Cpp_Code/main.cpp                    | 16 +++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ad9523.c b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ad9523.c
index a88e3dc..926b5ab 100644
--- a/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ad9523.c
+++ b/9_Firmware/9_1_Microcontroller/9_1_1_C_Cpp_Libraries/ad9523.c
@@ -704,7 +704,13 @@ int32_t ad9523_setup(struct ad9523_dev **device,
 			 AD9523_READBACK_CTRL,
 			 0x0);
 	ad9523_io_update(dev);
-	ad9523_calibrate(dev);
+	/* F-4.6: capture VCO calibration result. Without this, a failed
+	 * calibration (e.g. target VCO outside the 3.6-4.0 GHz band) is
+	 * silently swallowed and downstream relies on PLL2_LD alone in
+	 * ad9523_status(), which can pass spuriously. */
+	ret = ad9523_calibrate(dev);
+	if (ret < 0)
+		return ret;
 	ad9523_sync(dev);
 
 	*device = dev;
diff --git a/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp b/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp
index e226a7c..9cc45a1 100644
--- a/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp
+++ b/9_Firmware/9_1_Microcontroller/9_1_3_C_Cpp_Code/main.cpp
@@ -243,6 +243,11 @@ IWDG_HandleTypeDef hiwdg;
 ADF4382A_Manager lo_manager;
 extern SPI_HandleTypeDef hspi4;
 
+// AD9523 device handle. Retained at file scope so the recovery path can free
+// the previous instance before re-running setup (F-4.5: ad9523_setup() malloc's
+// a new dev + SPI desc each call; without retention each recovery cycle leaks).
+static struct ad9523_dev *g_ad9523_dev = NULL;
+
 //ADAR1000
 
 ADAR1000Manager adarManager;
@@ -1420,6 +1425,14 @@ static int configure_ad9523(void)
     int32_t ret;
     struct ad9523_dev *dev = NULL;
 
+    // F-4.5: recovery path re-runs configure_ad9523(); free the previous
+    // dev/SPI-desc allocation before letting setup() malloc a new pair.
+    if (g_ad9523_dev != NULL) {
+        DIAG("CLK", "Releasing previous ad9523_dev before re-setup");
+        ad9523_remove(g_ad9523_dev);
+        g_ad9523_dev = NULL;
+    }
+
     static struct ad9523_platform_data pdata;
     memset(&pdata, 0, sizeof(pdata));
 
@@ -1614,7 +1627,8 @@ static int configure_ad9523(void)
     ad9523_sync(dev);
     DIAG("CLK", "AD9523 configuration complete -- all outputs should be active");
 
-    // keep device pointer globally if needed (dev)
+    // F-4.5: retain the dev handle so recovery can free it before re-setup.
+    g_ad9523_dev = dev;
     return 0;
 }