Change in simtrace2[master]: firmware/sniffer: Add + use 16bit ringbuffer

laforge has submitted this change. ( https://gerrit.osmocom.org/c/simtrace2/+/30192 ) Change subject: firmware/sniffer: Add + use 16bit ringbuffer ...................................................................... firmware/sniffer: Add + use 16bit ringbuffer So far, we use a uint8_t ring buffer as "FIFO" between USART Rx interrupt and main context. That's fine for expressing the bytes we receive. However, if we also want to report USART errors synchronously in that stream, we actually need more bits to express those. Reporting USART errors via the ring buffer is the only way how the sniffer code can know in which TPDU the error occurred. Reporting them any other way (global variable, ...) would loose the timing relationship where in the received stream the error occurred. This change just changes the ringbuffer from 1024-entry 8bit to 512-entry 16bit and doesn't add any error reporting. Change-Id: Ifde054fbfe7f753b61e7d3409c56eca6e0faeb4b --- M firmware/libcommon/include/ringbuffer.h M firmware/libcommon/source/ringbuffer.c M firmware/libcommon/source/sniffer.c 3 files changed, 88 insertions(+), 6 deletions(-) Approvals: laforge: Looks good to me, approved; Verified Hoernchen: Looks good to me, but someone else must approve diff --git a/firmware/libcommon/include/ringbuffer.h b/firmware/libcommon/include/ringbuffer.h index 51af3b2..99fe96b 100644 --- a/firmware/libcommon/include/ringbuffer.h +++ b/firmware/libcommon/include/ringbuffer.h @@ -32,4 +32,22 @@ bool rbuf_is_empty(volatile ringbuf * rb); bool rbuf_is_full(volatile ringbuf * rb); + +/* same as above but with 16bit values instead of 8bit */ + +#define RING16_BUFLEN 512 + +typedef struct ringbuf16 { + uint16_t buf[RING16_BUFLEN]; + size_t ird; + size_t iwr; +} ringbuf16; + +void rbuf16_reset(volatile ringbuf16 * rb); +uint16_t rbuf16_read(volatile ringbuf16 * rb); +uint16_t rbuf16_peek(volatile ringbuf16 * rb); +int rbuf16_write(volatile ringbuf16 * rb, uint16_t item); +bool rbuf16_is_empty(volatile ringbuf16 * rb); +bool rbuf16_is_full(volatile ringbuf16 * rb); + #endif /* end of include guard: SIMTRACE_RINGBUF_H */ diff --git a/firmware/libcommon/source/ringbuffer.c b/firmware/libcommon/source/ringbuffer.c index 591d31f..d3690be 100644 --- a/firmware/libcommon/source/ringbuffer.c +++ b/firmware/libcommon/source/ringbuffer.c @@ -28,6 +28,16 @@ local_irq_restore(state); } +void rbuf16_reset(volatile ringbuf16 * rb) +{ + unsigned long state; + + local_irq_save(state); + rb->ird = 0; + rb->iwr = 0; + local_irq_restore(state); +} + uint8_t rbuf_read(volatile ringbuf * rb) { unsigned long state; @@ -41,21 +51,49 @@ return val; } +uint16_t rbuf16_read(volatile ringbuf16 * rb) +{ + unsigned long state; + uint16_t val; + + local_irq_save(state); + val = rb->buf[rb->ird]; + rb->ird = (rb->ird + 1) % RING16_BUFLEN; + local_irq_restore(state); + + return val; +} + uint8_t rbuf_peek(volatile ringbuf * rb) { return rb->buf[rb->ird]; } +uint16_t rbuf16_peek(volatile ringbuf16 * rb) +{ + return rb->buf[rb->ird]; +} + bool rbuf_is_empty(volatile ringbuf * rb) { return rb->ird == rb->iwr; } +bool rbuf16_is_empty(volatile ringbuf16 * rb) +{ + return rb->ird == rb->iwr; +} + static bool __rbuf_is_full(volatile ringbuf * rb) { return rb->ird == (rb->iwr + 1) % RING_BUFLEN; } +static bool __rbuf16_is_full(volatile ringbuf16 * rb) +{ + return rb->ird == (rb->iwr + 1) % RING16_BUFLEN; +} + bool rbuf_is_full(volatile ringbuf * rb) { unsigned long state; @@ -68,6 +106,18 @@ return rc; } +bool rbuf16_is_full(volatile ringbuf16 * rb) +{ + unsigned long state; + bool rc; + + local_irq_save(state); + rc = rb->ird == (rb->iwr + 1) % RING16_BUFLEN; + local_irq_restore(state); + + return rc; +} + int rbuf_write(volatile ringbuf * rb, uint8_t item) { unsigned long state; @@ -84,4 +134,18 @@ } } +int rbuf16_write(volatile ringbuf16 * rb, uint16_t item) +{ + unsigned long state; + local_irq_save(state); + if (!__rbuf16_is_full(rb)) { + rb->buf[rb->iwr] = item; + rb->iwr = (rb->iwr + 1) % RING16_BUFLEN; + local_irq_restore(state); + return 0; + } else { + local_irq_restore(state); + return -1; + } +} diff --git a/firmware/libcommon/source/sniffer.c b/firmware/libcommon/source/sniffer.c index 153c483..bea4d95 100644 --- a/firmware/libcommon/source/sniffer.c +++ b/firmware/libcommon/source/sniffer.c @@ -138,7 +138,7 @@ .state = USART_RCV, }; /*! Ring buffer to store sniffer communication data */ -static struct ringbuf sniff_buffer; +static struct ringbuf16 sniff_buffer; /* Flags to know is the card status changed (see SIMTRACE_MSGT_DT_SNIFF_CHANGE flags) */ static volatile uint32_t change_flags = 0; @@ -291,7 +291,7 @@ update_wt(10, 1, "RESET"); /* reset WT time-out */ break; case ISO7816_S_WAIT_ATR: - rbuf_reset(&sniff_buffer); /* reset buffer for new communication */ + rbuf16_reset(&sniff_buffer); /* reset buffer for new communication */ break; case ISO7816_S_IN_ATR: g_atr.atr_i = 0; @@ -835,7 +835,7 @@ /* Reset WT timer */ wt_remaining = g_wt; /* Store sniffed data into buffer (also clear interrupt */ - if (rbuf_write(&sniff_buffer, byte) != 0) + if (rbuf16_write(&sniff_buffer, byte) != 0) TRACE_ERROR("USART buffer full\n\r"); } @@ -941,7 +941,7 @@ PIO_EnableIt(&pin_rst); /* Clear ring buffer containing the sniffed data */ - rbuf_reset(&sniff_buffer); + rbuf16_reset(&sniff_buffer); /* Configure USART to as ISO-7816 slave communication to sniff communication */ ISO7816_Init(&sniff_usart, CLK_SLAVE); /* Only receive data when sniffing */ @@ -1007,8 +1007,8 @@ * is remaining */ /* Handle sniffed data */ - if (!rbuf_is_empty(&sniff_buffer)) { /* use if instead of while to let the main loop restart the watchdog */ - uint8_t byte = rbuf_read(&sniff_buffer); + if (!rbuf16_is_empty(&sniff_buffer)) { /* use if instead of while to let the main loop restart the watchdog */ + uint8_t byte = rbuf16_read(&sniff_buffer); /* Convert convention if required */ if (convention_convert) { byte = convention_convert_lut[byte]; -- To view, visit https://gerrit.osmocom.org/c/simtrace2/+/30192 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: simtrace2 Gerrit-Branch: master Gerrit-Change-Id: Ifde054fbfe7f753b61e7d3409c56eca6e0faeb4b Gerrit-Change-Number: 30192 Gerrit-PatchSet: 2 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: tsaitgaist <kredon(a)sysmocom.de> Gerrit-MessageType: merged