From db72836ed57663880b34363961fc24be82e1c7be Mon Sep 17 00:00:00 2001 From: Ingmar Splitt Date: Tue, 30 Apr 2024 12:06:08 +0200 Subject: [PATCH] improve message system --- software/firmware/include/commons.h | 11 ++-- software/firmware/pru0-programmer/main.c | 8 +-- software/firmware/pru0-shepherd-fw/main.c | 8 +-- software/firmware/pru1-shepherd-fw/main.c | 5 +- software/kernel-module/src/commons.h | 13 +++-- software/kernel-module/src/pru_msg_sys.c | 58 ++++++++++--------- software/kernel-module/src/pru_msg_sys.h | 3 +- software/kernel-module/src/pru_sync_control.c | 2 +- .../python-package/shepherd_sheep/commons.py | 8 +-- .../shepherd_sheep/shepherd_debug.py | 1 + .../shepherd_sheep/shepherd_emulator.py | 4 +- .../shepherd_sheep/shepherd_io.py | 8 --- 12 files changed, 61 insertions(+), 68 deletions(-) diff --git a/software/firmware/include/commons.h b/software/firmware/include/commons.h index 54b81acb..8644716d 100644 --- a/software/firmware/include/commons.h +++ b/software/firmware/include/commons.h @@ -58,9 +58,6 @@ enum MsgType MSG_DBG_FN_TESTS = 0xAFu, MSG_DBG_VSRC_HRV_P_INP = 0xB1u, // HRV + CNV in one go - /* KERNELSPACE (enum >=0xC0) */ - // STATUS - MSG_STATUS_RESTARTING_ROUTINE = 0xC0u, // ERROR MSG_ERROR = 0xE0u, MSG_ERR_MEMCORRUPTION = 0xE1u, @@ -72,9 +69,13 @@ enum MsgType MSG_ERR_TIMESTAMP = 0xE6u, MSG_ERR_SYNC_STATE_NOT_IDLE = 0xE7u, MSG_ERR_VALUE = 0xE8u, + + /* KERNELSPACE (enum >=0xF0) */ + // STATUS + MSG_STATUS_RESTARTING_ROUTINE = 0xF0u, // Routines - MSG_TEST = 0xEAu, - MSG_SYNC = 0xEBu + MSG_TEST_ROUTINE = 0xFAu, + MSG_SYNC_ROUTINE = 0xFBu }; /* Message IDs used in Mem-Protocol between PRUs and kernel module */ diff --git a/software/firmware/pru0-programmer/main.c b/software/firmware/pru0-programmer/main.c index 3fd73940..79d87d28 100644 --- a/software/firmware/pru0-programmer/main.c +++ b/software/firmware/pru0-programmer/main.c @@ -53,15 +53,15 @@ static bool_ft handle_kernel_com(volatile struct SharedMem *const shared_mem, ring_put(free_buffers_ptr, (uint8_t) msg_in.value[0]); return 1U; } - else if ((msg_in.type == MSG_TEST) && (msg_in.value[0] == 1)) + else if ((msg_in.type == MSG_TEST_ROUTINE) && (msg_in.value[0] == 1)) { // pipeline-test for msg-system - send_message(shared_mem, MSG_TEST, msg_in.value[0], 0); + send_message(shared_mem, MSG_TEST_ROUTINE, msg_in.value[0], 0); } - else if ((msg_in.type == MSG_TEST) && (msg_in.value[0] == 2)) + else if ((msg_in.type == MSG_TEST_ROUTINE) && (msg_in.value[0] == 2)) { // pipeline-test for msg-system - send_status(shared_mem, MSG_TEST, msg_in.value[0]); + send_status(shared_mem, MSG_TEST_ROUTINE, msg_in.value[0]); } else { send_message(shared_mem, MSG_ERR_INVLDCMD, msg_in.type, 0); } } diff --git a/software/firmware/pru0-shepherd-fw/main.c b/software/firmware/pru0-shepherd-fw/main.c index cfc2cb21..251b89fc 100644 --- a/software/firmware/pru0-shepherd-fw/main.c +++ b/software/firmware/pru0-shepherd-fw/main.c @@ -297,15 +297,15 @@ static bool_ft handle_kernel_com(volatile struct SharedMem *const shared_mem, ring_put(free_buffers_ptr, (uint8_t) msg_in.value[0]); return 1U; } - else if ((msg_in.type == MSG_TEST) && (msg_in.value[0] == 1)) + else if ((msg_in.type == MSG_TEST_ROUTINE) && (msg_in.value[0] == 1)) { // pipeline-test for msg-system - send_message(shared_mem, MSG_TEST, msg_in.value[0], 0); + send_message(shared_mem, MSG_TEST_ROUTINE, msg_in.value[0], 0); } - else if ((msg_in.type == MSG_TEST) && (msg_in.value[0] == 2)) + else if ((msg_in.type == MSG_TEST_ROUTINE) && (msg_in.value[0] == 2)) { // pipeline-test for msg-system - send_status(shared_mem, MSG_TEST, msg_in.value[0]); + send_status(shared_mem, MSG_TEST_ROUTINE, msg_in.value[0]); } else { send_message(shared_mem, MSG_ERR_INVLDCMD, msg_in.type, 0); } } diff --git a/software/firmware/pru1-shepherd-fw/main.c b/software/firmware/pru1-shepherd-fw/main.c index c8c5414b..52bc0004 100644 --- a/software/firmware/pru1-shepherd-fw/main.c +++ b/software/firmware/pru1-shepherd-fw/main.c @@ -83,11 +83,12 @@ static inline bool_ft receive_sync_reply(volatile struct SharedMem *const shared send_status(shared_mem, MSG_ERR_MEMCORRUPTION, 0); return 0; } - if (shared_mem->pru1_sync_inbox.type == MSG_TEST) + if (shared_mem->pru1_sync_inbox.type == MSG_TEST_ROUTINE) { // pipeline-test for msg-system shared_mem->pru1_sync_inbox.unread = 0; - send_status(shared_mem, MSG_TEST, shared_mem->pru1_sync_inbox.buffer_block_period); + send_status(shared_mem, MSG_TEST_ROUTINE, + shared_mem->pru1_sync_inbox.buffer_block_period); return 0; } // NOTE: do not overwrite external msg without thinking twice! sync-routine relies on that content diff --git a/software/kernel-module/src/commons.h b/software/kernel-module/src/commons.h index b90804be..468cb935 100644 --- a/software/kernel-module/src/commons.h +++ b/software/kernel-module/src/commons.h @@ -57,22 +57,23 @@ enum MsgType MSG_DBG_FN_TESTS = 0xAF, MSG_DBG_VSRC_HRV_P_INP = 0xB1, // HRV + CNV in one go - /* KERNELSPACE (enum >=0xC0) */ - // STATUS - MSG_STATUS_RESTARTING_ROUTINE = 0xC0, // ERROR MSG_ERROR = 0xE0u, MSG_ERR_MEMCORRUPTION = 0xE1u, MSG_ERR_BACKPRESSURE = 0xE2u, MSG_ERR_INCMPLT = 0xE3u, MSG_ERR_INVLDCMD = 0xE4u, - MSG_ERR_NOFREEBUF = 0xE5u, + MSG_ERR_NOFREEBUF = 0xE5u, // this one marks the end of emulation MSG_ERR_TIMESTAMP = 0xE6u, MSG_ERR_SYNC_STATE_NOT_IDLE = 0xE7u, MSG_ERR_VALUE = 0xE8u, + + /* KERNELSPACE (enum >=0xF0) */ + // STATUS + MSG_STATUS_RESTARTING_ROUTINE = 0xF0u, // Routines - MSG_TEST = 0xEAu, - MSG_SYNC = 0xEBu + MSG_TEST_ROUTINE = 0xFAu, + MSG_SYNC_ROUTINE = 0xFBu }; enum MsgID diff --git a/software/kernel-module/src/pru_msg_sys.c b/software/kernel-module/src/pru_msg_sys.c index 26b6c95b..c085b8c3 100644 --- a/software/kernel-module/src/pru_msg_sys.c +++ b/software/kernel-module/src/pru_msg_sys.c @@ -2,12 +2,6 @@ #include #include -#define USE_MTX -#ifdef USE_MTX - // NOTE: was deactivated for fixing trouble with realtime-kernel - #include -#endif - #include "pru_mem_interface.h" #include "pru_msg_sys.h" @@ -25,16 +19,20 @@ static void ring_init(struct RingBuffer *const buf) buf->end = 0u; buf->active = 0u; -#ifdef USE_MTX - mutex_init(&buf->mutex); -#endif + buf->mutex = 0u; } +static void mutex_lock(uint32_t *const mutex, const uint32_t num) +{ + while (*mutex != 0u); + *mutex = num; +} + +static void mutex_unlock(uint32_t *const mutex) { *mutex = 0u; } + static void ring_put(struct RingBuffer *const buf, const struct ProtoMsg *const element) { -#ifdef USE_MTX - mutex_lock(&buf->mutex); -#endif + mutex_lock(&buf->mutex, 100u); buf->ring[buf->end] = *element; @@ -48,23 +46,22 @@ static void ring_put(struct RingBuffer *const buf, const struct ProtoMsg *const /* fire warning - maybe not the best place to do this - could start an avalanche */ printk(KERN_ERR "shprd.k: FIFO of msg-system is full - lost oldest msg!"); } -#ifdef USE_MTX + mutex_unlock(&buf->mutex); -#endif } static uint8_t ring_get(struct RingBuffer *const buf, struct ProtoMsg *const element) { if (buf->active == 0) return 0; -#ifdef USE_MTX - mutex_lock(&buf->mutex); -#endif + + mutex_lock(&buf->mutex, 200u); + *element = buf->ring[buf->start]; if (++(buf->start) == MSG_FIFO_SIZE) buf->start = 0U; // fast modulo buf->active--; -#ifdef USE_MTX + mutex_unlock(&buf->mutex); -#endif + return 1; } @@ -118,13 +115,13 @@ void msg_sys_test(void) struct SyncMsg msg2; printk(KERN_INFO "shprd.k: test msg-pipelines between kM and PRUs -> triggering " "roundtrip-messages for pipeline 1-3"); - msg1.type = MSG_TEST; + msg1.type = MSG_TEST_ROUTINE; msg1.value[0] = 1; put_msg_to_pru(&msg1); // message-pipeline pru0 msg1.value[0] = 2; put_msg_to_pru(&msg1); // error-pipeline pru0 - msg2.type = MSG_TEST; + msg2.type = MSG_TEST_ROUTINE; msg2.buffer_block_period = 3; pru1_comm_send_sync_reply(&msg2); // error-pipeline pru1 } @@ -210,16 +207,17 @@ static enum hrtimer_restart coordinator_callback(struct hrtimer *timer_for_resta break; } - if (pru_msg.type < MSG_TEST) { ring_put(&msg_ringbuf_from_pru, &pru_msg); } + if (pru_msg.type <= 0xF0u) + { + // relay everything below kernelspace and also RESTARTING_ROUTINE + ring_put(&msg_ringbuf_from_pru, &pru_msg); + } - if (pru_msg.type >= MSG_STATUS_RESTARTING_ROUTINE) + if (pru_msg.type >= 0xE0u) { switch (pru_msg.type) { // NOTE: all MSG_ERR also get handed to python - case MSG_STATUS_RESTARTING_ROUTINE: - printk(KERN_INFO "shprd.pru%u: (re)starting main-routine", had_work & 1u); - break; case MSG_ERROR: printk(KERN_ERR "shprd.pru%u: general error (val=%u)", had_work & 1u, pru_msg.value[0]); @@ -246,14 +244,18 @@ static enum hrtimer_restart coordinator_callback(struct hrtimer *timer_for_resta printk(KERN_ERR "shprd.pru%u: content of msg failed test (val=%u)", had_work & 1u, pru_msg.value[0]); break; - case MSG_TEST: + + case MSG_STATUS_RESTARTING_ROUTINE: + printk(KERN_INFO "shprd.pru%u: (re)starting main-routine", had_work & 1u); + break; + case MSG_TEST_ROUTINE: printk(KERN_INFO "shprd.k: [test passed] received answer from " "pru%u / pipeline %u", had_work & 1u, pru_msg.value[0]); break; default: /* these are all handled in userspace and will be passed by sys-fs */ - printk(KERN_ERR "shprd.k: received invalid command / msg-type (%u) " + printk(KERN_ERR "shprd.k: received invalid command / msg-type (%02X) " "from pru%u", pru_msg.type, had_work & 1u); } diff --git a/software/kernel-module/src/pru_msg_sys.h b/software/kernel-module/src/pru_msg_sys.h index df52bd7c..908ff25f 100644 --- a/software/kernel-module/src/pru_msg_sys.h +++ b/software/kernel-module/src/pru_msg_sys.h @@ -2,7 +2,6 @@ #define SRC_MSG_PRU_H #include "commons.h" -#include struct RingBuffer { @@ -10,7 +9,7 @@ struct RingBuffer uint32_t start; // TODO: these can be smaller (like u8), at least in documentation uint32_t end; uint32_t active; - struct mutex mutex; + uint32_t mutex; }; void put_msg_to_pru(const struct ProtoMsg *const element); diff --git a/software/kernel-module/src/pru_sync_control.c b/software/kernel-module/src/pru_sync_control.c index 2d9d59a9..bc3b1c9c 100644 --- a/software/kernel-module/src/pru_sync_control.c +++ b/software/kernel-module/src/pru_sync_control.c @@ -461,7 +461,7 @@ int sync_loop(struct SyncMsg *const sync_reply, const struct ProtoMsg *const syn if (sync_data->clock_corr < -80000) sync_data->clock_corr = -80000; /* determine corrected loop_ticks for next buffer_block */ - sync_reply->type = MSG_SYNC; + sync_reply->type = MSG_SYNC_ROUTINE; sync_reply->buffer_block_period = TIMER_BASE_PERIOD + sync_data->clock_corr; sync_data->previous_period = sync_reply->buffer_block_period; sync_reply->analog_sample_period = (sync_reply->buffer_block_period / ADC_SAMPLES_PER_BUFFER); diff --git a/software/python-package/shepherd_sheep/commons.py b/software/python-package/shepherd_sheep/commons.py index 2b147066..8e7971d7 100644 --- a/software/python-package/shepherd_sheep/commons.py +++ b/software/python-package/shepherd_sheep/commons.py @@ -33,10 +33,6 @@ MSG_DBG_FN_TESTS = 0xAF MSG_DBG_VSRC_HRV_P_INP = 0xB1 -# NOTE: below messages were previously exclusive to kernel space - -MSG_STATUS_RESTARTING_ROUTINE = 0xC0 - # TODO: these 9 lines below are replaced by the following dict MSG_ERROR = 0xE0 MSG_ERR_MEMCORRUPTION = 0xE1 @@ -48,8 +44,8 @@ MSG_ERR_SYNC_STATE_NOT_IDLE = 0xE7 MSG_ERR_VALUE = 0xE8 -MSG_TEST = 0xEA -MSG_SYNC = 0xEB +# NOTE: below messages are exclusive to kernel space +MSG_STATUS_RESTARTING_ROUTINE = 0xF0 pru_errors: dict[int, str] = { 0xE0: "General (unspecified) PRU-error [MSG_ERROR]", diff --git a/software/python-package/shepherd_sheep/shepherd_debug.py b/software/python-package/shepherd_sheep/shepherd_debug.py index a792aaea..eebb5344 100644 --- a/software/python-package/shepherd_sheep/shepherd_debug.py +++ b/software/python-package/shepherd_sheep/shepherd_debug.py @@ -438,6 +438,7 @@ def process_programming_messages(self) -> None: while True: msg_type, values = self._get_msg(5) if msg_type != commons.MSG_PGM_ERROR_WRITE: + # TODO: that should trigger an error log.error( "PROGRAMMER-WRITE-ERROR: ihex to target @%s, data=%d [%s]", f"0x{values[0]:X}", diff --git a/software/python-package/shepherd_sheep/shepherd_emulator.py b/software/python-package/shepherd_sheep/shepherd_emulator.py index 6682f005..f74b1ca6 100644 --- a/software/python-package/shepherd_sheep/shepherd_emulator.py +++ b/software/python-package/shepherd_sheep/shepherd_emulator.py @@ -236,7 +236,7 @@ def run(self) -> None: idx, emu_buf = self.get_buffer(verbose=self.verbose_extra) if emu_buf.timestamp_ns / 1e9 >= ts_end: - log.debug("FINISHED! Out of bound timestamp collected -> begin to exit now") + log.warning("UNEXPECTED! Out of bound timestamp collected -> begin to exit now") break if self.writer is not None: @@ -257,7 +257,7 @@ def run(self) -> None: while True: idx, emu_buf = self.get_buffer(verbose=self.verbose_extra) if emu_buf.timestamp_ns / 1e9 >= ts_end: - log.debug("FINISHED! Out of bound timestamp collected -> begin to exit now") + log.warning("UNEXPECTED! Out of bound timestamp collected -> begin to exit now") return if self.writer is not None: self.writer.write_buffer(emu_buf) diff --git a/software/python-package/shepherd_sheep/shepherd_io.py b/software/python-package/shepherd_sheep/shepherd_io.py index 100a75d0..0c0f54e9 100644 --- a/software/python-package/shepherd_sheep/shepherd_io.py +++ b/software/python-package/shepherd_sheep/shepherd_io.py @@ -538,14 +538,6 @@ def get_buffer( log.info("Received cmd to print: %d", value) continue - if msg_type == commons.MSG_TEST: - log.debug("Received test-message from PRU: %d", value) - continue - - if msg_type == commons.MSG_SYNC: - log.debug("Received sync-message from PRU: %d", value) - continue - if msg_type == commons.MSG_STATUS_RESTARTING_ROUTINE: log.debug("PRU is restarting its main routine, val=%d", value) continue