Merge pull request #18701 from benpicco/sys/checksum-crc16_fast

sys/checksum: add CRC-16 implementation without lookup table
RIOT-OS · Oct 7, 2022 · 24f7d20 · 24f7d20
2 parents 213c35b + 98e18ed
commit 24f7d20
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 19 deletions.
diff --git a/drivers/dose/Kconfig b/drivers/dose/Kconfig
@@ -28,6 +28,7 @@ menuconfig MODULE_DOSE
     depends on HAS_PERIPH_GPIO
     depends on HAS_PERIPH_GPIO_IRQ
     depends on HAS_PERIPH_UART
+    select MODULE_CHECKSUM
     select MODULE_CHUNKED_RINGBUFFER
     select MODULE_EUI_PROVIDER
     select MODULE_IOLIST

diff --git a/drivers/dose/Makefile.dep b/drivers/dose/Makefile.dep
@@ -7,6 +7,7 @@ ifneq (,$(filter dose_watchdog,$(USEMODULE)))
   FEATURES_REQUIRED += periph_timer_periodic
 endif
 
+USEMODULE += checksum
 USEMODULE += chunked_ringbuffer
 USEMODULE += eui_provider
 USEMODULE += iolist

diff --git a/drivers/dose/dose.c b/drivers/dose/dose.c
@@ -27,6 +27,7 @@
 #include "irq.h"
 #include "periph/timer.h"
 
+#include "checksum/crc16_ccitt.h"
 #include "net/eui_provider.h"
 #include "net/netdev/eth.h"
 #include "timex.h"
@@ -38,7 +39,6 @@
 #error "DOSE_TIMER_DEV needs to be set by the board"
 #endif
 
-static uint16_t crc16_update(uint16_t crc, uint8_t octet);
 static dose_signal_t state_transit_blocked(dose_t *ctx, dose_signal_t signal);
 static dose_signal_t state_transit_idle(dose_t *ctx, dose_signal_t signal);
 static dose_signal_t state_transit_recv(dose_t *ctx, dose_signal_t signal);
@@ -59,22 +59,10 @@ static int _init(netdev_t *dev);
 static void _poweron(dose_t *dev);
 static void _poweroff(dose_t *dev, dose_state_t sleep_state);
 
-static uint16_t crc16_update(uint16_t crc, uint8_t octet)
-{
-    crc = (uint8_t)(crc >> 8) | (crc << 8);
-    crc ^= octet;
-    crc ^= (uint8_t)(crc & 0xff) >> 4;
-    crc ^= (crc << 8) << 4;
-    crc ^= ((crc & 0xff) << 4) << 1;
-    return crc;
-}
-
 static void _crc_cb(void *ctx, uint8_t *data, size_t len)
 {
     uint16_t *crc = ctx;
-    for (uint8_t *end = data + len; data != end; ++data) {
-        *crc = crc16_update(*crc, *data);
-    }
+    *crc = crc16_ccitt_false_update(*crc, data, len);
 }
 
 static void _init_standby(dose_t *ctx, const dose_params_t *params)
@@ -571,15 +559,13 @@ static int _send(netdev_t *dev, const iolist_t *iolist)
         size_t n = iol->iol_len;
         pktlen += n;
         uint8_t *ptr = iol->iol_base;
+        crc = crc16_ccitt_false_update(crc, ptr, n);
         while (n--) {
             /* Send data octet */
             if (send_data_octet(ctx, *ptr)) {
                 goto collision;
             }
 
-            /* Update CRC */
-            crc = crc16_update(crc, *ptr);
-
             ptr++;
         }
     }

diff --git a/makefiles/pseudomodules.inc.mk b/makefiles/pseudomodules.inc.mk
@@ -45,6 +45,7 @@ PSEUDOMODULES += cortexm_fpu
 PSEUDOMODULES += cortexm_svc
 PSEUDOMODULES += cpp
 PSEUDOMODULES += cpu_check_address
+PSEUDOMODULES += crc16_fast
 PSEUDOMODULES += crc32_fast
 PSEUDOMODULES += credman_load
 PSEUDOMODULES += dbgpin

diff --git a/sys/Makefile.dep b/sys/Makefile.dep
@@ -49,6 +49,10 @@ ifneq (,$(filter congure_reno_methods,$(USEMODULE)))
   USEMODULE += seq
 endif
 
+ifneq (,$(filter crc16_fast,$(USEMODULE)))
+  USEMODULE += checksum
+endif
+
 ifneq (,$(filter crc32_fast,$(USEMODULE)))
   USEMODULE += checksum
 endif

diff --git a/sys/checksum/crc16_ccitt.c b/sys/checksum/crc16_ccitt.c
@@ -21,6 +21,8 @@
 #include <stdint.h>
 #include <stdlib.h>
 
+#include "byteorder.h"
+#include "kernel_defines.h"
 #include "checksum/crc16_ccitt.h"
 
 static const uint16_t crc_ccitt_lookuptable[256] = {
@@ -96,7 +98,16 @@ static const uint16_t _crc16_ccitt_false_lookuptable[256] = {
 uint16_t crc16_ccitt_kermit_update(uint16_t crc, const unsigned char *buf, size_t len)
 {
     while (len--) {
-        crc = (crc >> 8) ^ crc_ccitt_lookuptable[(crc ^ (*buf++)) & 0xff];
+        uint8_t e = crc ^= *buf++;
+        if (IS_USED(MODULE_CRC16_FAST)) {
+            crc = (crc >> 8) ^ crc_ccitt_lookuptable[e];
+        } else {
+            uint8_t f = e ^ (e << 4);
+            crc = (crc >> 8)
+                ^ ((uint16_t)f << 8)
+                ^ ((uint16_t)f << 3)
+                ^ ((uint16_t)f >> 4);
+        }
     }
 
     return crc;
@@ -115,7 +126,14 @@ uint16_t crc16_ccitt_mcrf4xx_calc(const unsigned char *buf, size_t len)
 uint16_t crc16_ccitt_false_update(uint16_t crc, const unsigned char *buf, size_t len)
 {
     while (len--) {
-        crc = ((crc << 8) ^ _crc16_ccitt_false_lookuptable[((crc >> 8) ^ ((*buf++) & 0x00FF))]);
+        crc = byteorder_swaps(crc) ^ *buf++;
+        if (IS_USED(MODULE_CRC16_FAST)) {
+            crc = (crc & 0xFF00) ^ _crc16_ccitt_false_lookuptable[crc & 0xFF];
+        } else {
+            crc ^= (uint8_t)(crc & 0xff) >> 4;
+            crc ^= (crc << 8) << 4;
+            crc ^= ((crc & 0xff) << 4) << 1;
+        }
     }
 
     return crc;

diff --git a/sys/include/checksum/crc16_ccitt.h b/sys/include/checksum/crc16_ccitt.h
@@ -18,6 +18,9 @@
  *              do (and is thus also far more memory efficient). Its caveat
  *              however is that it is slower by about factor 8 than these versions.
  *
+ *              @note enable the `crc32_fast` module for a look-up table based
+ *              implementation that trades code size for speed.
+ *
  * @{
  * @file
  * @author      Ludwig Knüpfer <ludwig.knuepfer@fu-berlin.de>