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 <lafo...@osmocom.org>
Gerrit-Reviewer: Hoernchen <ew...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: tsaitgaist <kre...@sysmocom.de>
Gerrit-MessageType: merged

Reply via email to