lib: nrf_modem: fixes for flash trace backend

Fixes for trace flash backend.

Signed-off-by: Eivind Jølsgard <[email protected]>

Author: eivindj-nordic

lib/nrf_modem_lib/nrf_modem_lib_trace.c

@@ -261,7 +261,10 @@ static int trace_fragment_write(struct nrf_modem_trace_data *frag)
 		}
 
 		if (ret < 0) {
-			LOG_ERR("trace_backend.write failed with err: %d", ret);
+			if (ret != -ENOSPC) {
+				LOG_ERR("trace_backend.write failed with err: %d", ret);
+			}
+
 			return ret;
 		}
 
@@ -315,6 +318,7 @@ void trace_thread_handler(void)
 		}
 
 		for (int i = 0; i < n_frags; i++) {
+retry:
 			err = trace_fragment_write(&frags[i]);
 			switch (err) {
 			case 0:
@@ -329,8 +333,7 @@ void trace_thread_handler(void)
 				k_sem_give(&trace_done_sem);
 				k_sem_take(&trace_clear_sem, K_FOREVER);
 				/* Try the same fragment again */
-				i--;
-				continue;
+				goto retry;
 
 			case -ENOSR:
 				if (k_sem_take(&modem_trace_level_sem, K_NO_WAIT) != 0) {
@@ -348,8 +351,7 @@ void trace_thread_handler(void)
 				k_sem_give(&modem_trace_level_sem);
 
 				/* Try the same fragment again */
-				i--;
-				continue;
+				goto retry;
 
 			default:
 				/* Irrecoverable error */
@@ -497,7 +499,12 @@ int nrf_modem_lib_trace_read(uint8_t *buf, size_t len)
 		UPDATE_TRACE_BYTES_READ(read);
 		/* Traces are read, we can attempt to write more. */
 		has_space = true;
+/* Flash backend needs to wait for a sector to be cleared and will give the semaphore instead.
+ * This should be cleaned up with a separate API, but we declare the semaphore as extern for now.
+ */
+#if !defined(CONFIG_NRF_MODEM_LIB_TRACE_BACKEND_FLASH)
 		k_sem_give(&trace_clear_sem);
+#endif
 	}
 
 	return read;

lib/nrf_modem_lib/trace_backends/flash/flash.c

@@ -36,6 +36,11 @@ static struct fcb trace_fcb = {
 	.f_flags = FCB_FLAGS_CRC_DISABLED,
 };
 
+/* Flash backend needs to wait for a sector to be cleared and will give the semaphore instead.
+ * This should be cleaned up with a separate API, but we declare the semaphore as extern for now.
+ */
+extern struct k_sem trace_clear_sem;
+
 /* Store in __noinit RAM to perserve in warm boot. */
 static __noinit uint32_t magic;
 static __noinit size_t read_offset;
@@ -116,8 +121,10 @@ static int buffer_flush_to_flash(void)
 		}
 
 		if (err) {
-			LOG_ERR("fcb_append failed, err %d", err);
-			err = -ENOSPC;
+			if (err != -ENOSPC) {
+				LOG_ERR("fcb_append failed, err %d", err);
+			}
+
 			goto out;
 		}
 	}
@@ -189,14 +196,16 @@ int trace_backend_init(trace_backend_processed_cb trace_processed_cb)
 
 	/* After a cold boot the magic will contain random values. */
 	if (magic != TRACE_MAGIC_INITIALIZED) {
-		LOG_DBG("Initializing");
+		LOG_DBG("Trace magic not found, initializing");
 		read_offset = 0;
 		trace_bytes_unread = 0;
 		flash_buf_written = 0;
 		memset(&loc, 0, sizeof(loc));
 		sector = NULL;
 		magic = TRACE_MAGIC_INITIALIZED;
 		trace_flash_erase();
+	} else {
+		LOG_DBG("Trace magic found, skipping initialization");
 	}
 
 	uint32_t f_sector_cnt = sizeof(trace_flash_sectors) / sizeof(struct flash_sector);
@@ -266,6 +275,7 @@ static int read_from_offset(void *buf, size_t len)
 int trace_backend_read(void *buf, size_t len)
 {
 	int err;
+	size_t ret;
 	size_t to_read = 0;
 
 	if (!is_initialized) {
@@ -314,25 +324,32 @@ int trace_backend_read(void *buf, size_t len)
 	err = read_from_offset(buf, len);
 
 out:
+	ret = err;
+
 	/* Erase if done with previous sector. */
 	if (sector && (sector != loc.fe_sector)) {
 		err = fcb_rotate(&trace_fcb);
 		if (err) {
+			LOG_ERR("Failed to erase read sector, err %d", err);
 			k_sem_give(&fcb_sem);
-			return err;
+			/* Return what we have read */
+			return ret;
 		}
+
+		k_sem_give(&trace_clear_sem);
 	}
 
 	sector = loc.fe_sector;
 
 	k_sem_give(&fcb_sem);
-	return err;
+	return ret;
 }
 
 static int stream_write(const void *buf, size_t len)
 {
 	int ret;
 	int written;
+	size_t written_total = 0;
 	size_t bytes_left = len;
 	const uint8_t *bytes = buf;
 
@@ -342,36 +359,47 @@ static int stream_write(const void *buf, size_t len)
 
 	while (bytes_left) {
 		written = buffer_append(&bytes[len - bytes_left], bytes_left);
-		if (written != bytes_left) {
+		written_total += written;
+
+		if (flash_buf_written >= sizeof(flash_buf)) {
 			ret = buffer_flush_to_flash();
 			if (ret) {
 				LOG_ERR("buffer_flush_to_flash error %d", ret);
+				if (written_total) {
+					ret = trace_processed_callback(written);
+					if (ret < 0) {
+						LOG_ERR("trace_processed_callback failed: %d", ret);
+						return ret;
+					}
+					return written_total;
+				}
 				return ret;
 			}
 		}
 		if (written > 0) {
 			bytes_left -= written;
 			ret = trace_processed_callback(written);
 			if (ret < 0) {
-				LOG_ERR("trace_processed_callback failed: %d", ret);
+				LOG_ERR("trace_processed_callback 2 failed: %d", ret);
 				return ret;
 			}
 		}
 	}
 
-	return 0;
+	return written_total;
 }
 
 int trace_backend_write(const void *data, size_t len)
 {
 	int write_ret = stream_write(data, len);
 
 	if (write_ret < 0) {
-		LOG_ERR("write failed: %d", write_ret);
-		return write_ret;
+		if (write_ret != -ENOSPC) {
+			LOG_ERR("write failed: %d", write_ret);
+		}
 	}
 
-	return (int)len;
+	return write_ret;
 }
 
 int trace_backend_clear(void)