From abb1738e08e66587d09801f9db9ff54ea51f6cd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eivind=20J=C3=B8lsgard?= Date: Tue, 29 Oct 2024 15:38:37 +0100 Subject: [PATCH] lib: nrf_modem: fixes for flash trace backend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes for trace flash backend. Signed-off-by: Eivind Jølsgard --- lib/nrf_modem_lib/nrf_modem_lib_trace.c | 17 +++++-- .../trace_backends/flash/flash.c | 50 +++++++++++++++---- 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/lib/nrf_modem_lib/nrf_modem_lib_trace.c b/lib/nrf_modem_lib/nrf_modem_lib_trace.c index e631eed26083..f2e50b58f48e 100644 --- a/lib/nrf_modem_lib/nrf_modem_lib_trace.c +++ b/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; diff --git a/lib/nrf_modem_lib/trace_backends/flash/flash.c b/lib/nrf_modem_lib/trace_backends/flash/flash.c index 4184dcf5f69c..e3b6f04a4466 100644 --- a/lib/nrf_modem_lib/trace_backends/flash/flash.c +++ b/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,7 +196,7 @@ 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; @@ -197,6 +204,8 @@ int trace_backend_init(trace_backend_processed_cb trace_processed_cb) 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,10 +359,20 @@ 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; } } @@ -353,13 +380,13 @@ static int stream_write(const void *buf, size_t len) 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) @@ -367,11 +394,12 @@ 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)