fs: fcb: add option to disable CRC for fcb entries

Add option to disable CRC for fcb entries. This improves the write
throughput significantly at the cost of not detecting corrupted data
in flash. This is beneficial for aplications that needs the extra
write throughput, where error detection is done elsewhere.

Allow the FCB entries in flash to have a valid CRC when CRC is
disabled in the FCB. This allows existing solutions to disable
CRC checking, while keeping the CRC areas intact. Note that this
is a one-way option.

Fixes #53707

Signed-off-by: Eivind Jølsgard <eivind.jolsgard@nordicsemi.no>

doc/releases/release-notes-3.3.rst

@@ -504,6 +504,9 @@ Libraries / Subsystems
   * Added new API call `fs_mkfs`.
   * Added new sample `samples/subsys/fs/format`.
   * FAT FS driver has been updated to version 0.15 w/patch1.
+  * Added the option to disable CRC checking in :ref:`fcb_api` by enabling the
+    Kconfig option :kconfig:option:`CONFIG_FCB_ALLOW_FIXED_ENDMARKER`
+    and setting the `FCB_FLAGS_CRC_DISABLED` flag in the :c:struct:`fcb` struct.
 
 * Management
 

include/zephyr/fs/fcb.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017-2020 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -14,6 +14,7 @@
 #include <limits.h>
 
 #include <zephyr/storage/flash_map.h>
+#include <zephyr/sys/util_macro.h>
 
 #include <zephyr/kernel.h>
 
@@ -76,6 +77,11 @@ struct fcb_entry_ctx {
 	/**< Flash area where the entry is placed */
 };
 
+/**
+ * @brief Flag to disable CRC for the fcb_entries in flash.
+ */
+#define FCB_FLAGS_CRC_DISABLED BIT(0)
+
 /**
  * @brief FCB instance structure
  *
@@ -129,6 +135,10 @@ struct fcb {
 	/**< The value flash takes when it is erased. This is read from
 	 * flash parameters and initialized upon call to fcb_init.
 	 */
+#ifdef CONFIG_FCB_ALLOW_FIXED_ENDMARKER
+	const uint8_t f_flags;
+	/**< Flags for configuring the FCB. */
+#endif
 };
 
 /**

subsys/fs/fcb/Kconfig

@@ -1,6 +1,6 @@
 # Flash Circular Buffer module
 
-# Copyright (c) 2017 Nordic Semiconductor ASA
+# Copyright (c) 2017-2023 Nordic Semiconductor ASA
 # SPDX-License-Identifier: Apache-2.0
 
 #
@@ -13,3 +13,13 @@ config FCB
 	select CRC
 	help
 	  Enable support of Flash Circular Buffer.
+
+if FCB
+
+config FCB_ALLOW_FIXED_ENDMARKER
+	bool "Allow FCB instances to have a fixed endmarker"
+	help
+	  This allows the FCB instances to disable CRC checks in
+	  favor of increased write throughput.
+
+endif

subsys/fs/fcb/fcb_append.c

@@ -121,18 +121,18 @@ int
 fcb_append_finish(struct fcb *fcb, struct fcb_entry *loc)
 {
 	int rc;
-	uint8_t crc8[fcb->f_align];
+	uint8_t em[fcb->f_align];
 	off_t off;
 
-	(void)memset(crc8, 0xFF, sizeof(crc8));
+	(void)memset(em, 0xFF, sizeof(em));
 
-	rc = fcb_elem_crc8(fcb, loc, &crc8[0]);
+	rc = fcb_elem_endmarker(fcb, loc, &em[0]);
 	if (rc) {
 		return rc;
 	}
 	off = loc->fe_data_off + fcb_len_in_flash(fcb, loc->fe_data_len);
 
-	rc = fcb_flash_write(fcb, loc->fe_sector, off, crc8, fcb->f_align);
+	rc = fcb_flash_write(fcb, loc->fe_sector, off, em, fcb->f_align);
 	if (rc) {
 		return -EIO;
 	}

subsys/fs/fcb/fcb_elem_info.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -10,12 +10,14 @@
 #include <zephyr/fs/fcb.h>
 #include "fcb_priv.h"
 
+#define FCB_FIXED_ENDMARKER 0xab
+
 /*
  * Given offset in flash sector, fill in rest of the fcb_entry, and crc8 over
  * the data.
  */
-int
+static int
-fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *c8p)
+fcb_elem_crc8(struct fcb *_fcb, struct fcb_entry *loc, uint8_t *c8p)
 {
 	uint8_t tmp_str[FCB_TMP_BUF_SZ];
 	int cnt;
@@ -29,16 +31,17 @@ fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *c8p)
 	if (loc->fe_elem_off + 2 > loc->fe_sector->fs_size) {
 		return -ENOTSUP;
 	}
-	rc = fcb_flash_read(fcb, loc->fe_sector, loc->fe_elem_off, tmp_str, 2);
+
+	rc = fcb_flash_read(_fcb, loc->fe_sector, loc->fe_elem_off, tmp_str, 2);
 	if (rc) {
 		return -EIO;
 	}
 
-	cnt = fcb_get_len(fcb, tmp_str, &len);
+	cnt = fcb_get_len(_fcb, tmp_str, &len);
 	if (cnt < 0) {
 		return cnt;
 	}
-	loc->fe_data_off = loc->fe_elem_off + fcb_len_in_flash(fcb, cnt);
+	loc->fe_data_off = loc->fe_elem_off + fcb_len_in_flash(_fcb, cnt);
 	loc->fe_data_len = len;
 
 	crc8 = CRC8_CCITT_INITIAL_VALUE;
@@ -52,7 +55,7 @@ fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *c8p)
 			blk_sz = sizeof(tmp_str);
 		}
 
-		rc = fcb_flash_read(fcb, loc->fe_sector, off, tmp_str, blk_sz);
+		rc = fcb_flash_read(_fcb, loc->fe_sector, off, tmp_str, blk_sz);
 		if (rc) {
 			return -EIO;
 		}
@@ -63,25 +66,83 @@ fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *c8p)
 	return 0;
 }
 
-int fcb_elem_info(struct fcb *fcb, struct fcb_entry *loc)
+#if IS_ENABLED(CONFIG_FCB_ALLOW_FIXED_ENDMARKER)
+/* Given the offset in flash sector, calculate the FCB entry data offset and size, and set
+ * the fixed endmarker.
+ */
+static int
+fcb_elem_endmarker_fixed(struct fcb *_fcb, struct fcb_entry *loc, uint8_t *em)
 {
+	uint8_t tmp_str[2];
+	int cnt;
+	uint16_t len;
 	int rc;
-	uint8_t crc8;
-	uint8_t fl_crc8;
-	off_t off;
 
-	rc = fcb_elem_crc8(fcb, loc, &crc8);
+	if (loc->fe_elem_off + 2 > loc->fe_sector->fs_size) {
-	if (rc) {
+		return -ENOTSUP;
-		return rc;
 	}
-	off = loc->fe_data_off + fcb_len_in_flash(fcb, loc->fe_data_len);
 
-	rc = fcb_flash_read(fcb, loc->fe_sector, off, &fl_crc8, sizeof(fl_crc8));
+	rc = fcb_flash_read(_fcb, loc->fe_sector, loc->fe_elem_off, tmp_str, 2);
 	if (rc) {
 		return -EIO;
 	}
 
-	if (fl_crc8 != crc8) {
+	cnt = fcb_get_len(_fcb, tmp_str, &len);
+	if (cnt < 0) {
+		return cnt;
+	}
+	loc->fe_data_off = loc->fe_elem_off + fcb_len_in_flash(_fcb, cnt);
+	loc->fe_data_len = len;
+
+	*em = FCB_FIXED_ENDMARKER;
+	return 0;
+}
+#endif /* IS_ENABLED(CONFIG_FCB_ALLOW_FIXED_ENDMARKER) */
+
+/* Given the offset in flash sector, calculate the FCB entry data offset and size, and calculate
+ * the expected endmarker.
+ */
+int
+fcb_elem_endmarker(struct fcb *_fcb, struct fcb_entry *loc, uint8_t *em)
+{
+#if IS_ENABLED(CONFIG_FCB_ALLOW_FIXED_ENDMARKER)
+	if (_fcb->f_flags & FCB_FLAGS_CRC_DISABLED) {
+		return fcb_elem_endmarker_fixed(_fcb, loc, em);
+	}
+#endif /* IS_ENABLED(CONFIG_FCB_ALLOW_FIXED_ENDMARKER) */
+
+	return fcb_elem_crc8(_fcb, loc, em);
+}
+
+/* Given the offset in flash sector, calculate the FCB entry data offset and size, and verify that
+ * the FCB entry endmarker is correct.
+ */
+int fcb_elem_info(struct fcb *_fcb, struct fcb_entry *loc)
+{
+	int rc;
+	uint8_t em;
+	uint8_t fl_em;
+	off_t off;
+
+	rc = fcb_elem_endmarker(_fcb, loc, &em);
+	if (rc) {
+		return rc;
+	}
+	off = loc->fe_data_off + fcb_len_in_flash(_fcb, loc->fe_data_len);
+
+	rc = fcb_flash_read(_fcb, loc->fe_sector, off, &fl_em, sizeof(fl_em));
+	if (rc) {
+		return -EIO;
+	}
+
+	if (IS_ENABLED(CONFIG_FCB_ALLOW_FIXED_ENDMARKER) && (fl_em != em)) {
+		rc = fcb_elem_crc8(_fcb, loc, &em);
+		if (rc) {
+			return rc;
+		}
+	}
+
+	if (fl_em != em) {
 		return -EBADMSG;
 	}
 	return 0;

subsys/fs/fcb/fcb_priv.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017-2020 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -70,7 +70,7 @@ struct flash_sector *fcb_getnext_sector(struct fcb *fcb,
 int fcb_getnext_nolock(struct fcb *fcb, struct fcb_entry *loc);
 
 int fcb_elem_info(struct fcb *fcb, struct fcb_entry *loc);
-int fcb_elem_crc8(struct fcb *fcb, struct fcb_entry *loc, uint8_t *crc8p);
+int fcb_elem_endmarker(struct fcb *fcb, struct fcb_entry *loc, uint8_t *crc8p);
 
 int fcb_sector_hdr_init(struct fcb *fcb, struct flash_sector *sector, uint16_t id);
 int fcb_sector_hdr_read(struct fcb *fcb, struct flash_sector *sector,

tests/subsys/fs/fcb/prj.conf

@@ -5,5 +5,6 @@ CONFIG_FLASH_PAGE_LAYOUT=y
 CONFIG_FLASH_MAP=y
 CONFIG_ARM_MPU=n
 CONFIG_FCB=y
+CONFIG_FCB_ALLOW_FIXED_ENDMARKER=y
 CONFIG_SOC_FLASH_NRF_EMULATE_ONE_BYTE_WRITE_ACCESS=y
 CONFIG_ZTEST_NEW_API=y

tests/subsys/fs/fcb/prj_native_posix.conf

@@ -4,4 +4,5 @@ CONFIG_FLASH=y
 CONFIG_FLASH_PAGE_LAYOUT=y
 CONFIG_FLASH_MAP=y
 CONFIG_FCB=y
+CONFIG_FCB_ALLOW_FIXED_ENDMARKER=y
 CONFIG_ZTEST_NEW_API=y

tests/subsys/fs/fcb/prj_native_posix_64.conf

@@ -4,4 +4,5 @@ CONFIG_FLASH=y
 CONFIG_FLASH_PAGE_LAYOUT=y
 CONFIG_FLASH_MAP=y
 CONFIG_FCB=y
+CONFIG_FCB_ALLOW_FIXED_ENDMARKER=y
 CONFIG_ZTEST_NEW_API=y

tests/subsys/fs/fcb/prj_nucleo_h743zi.conf

@@ -6,7 +6,7 @@ CONFIG_FLASH_PAGE_LAYOUT=y
 CONFIG_FLASH_MAP=y
 CONFIG_ARM_MPU=n
 CONFIG_FCB=y
-
+CONFIG_FCB_ALLOW_FIXED_ENDMARKER=y
 CONFIG_MAIN_STACK_SIZE=4096
 CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=4096
 CONFIG_ZTEST_NEW_API=y

tests/subsys/fs/fcb/prj_qemu_x86.conf

@@ -4,5 +4,6 @@ CONFIG_FLASH=y
 CONFIG_FLASH_PAGE_LAYOUT=y
 CONFIG_FLASH_MAP=y
 CONFIG_FCB=y
+CONFIG_FCB_ALLOW_FIXED_ENDMARKER=y
 CONFIG_FLASH_SIMULATOR_UNALIGNED_READ=y
 CONFIG_ZTEST_NEW_API=y

tests/subsys/fs/fcb/src/fcb_test.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017-2020 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -24,6 +24,7 @@ extern "C" {
 #define TEST_FCB_FLASH_AREA_ID		FIXED_PARTITION_ID(TEST_FCB_FLASH_AREA)
 
 extern struct fcb test_fcb;
+extern struct fcb test_fcb_crc_disabled;
 
 extern struct flash_sector test_fcb_sector[];
 

tests/subsys/fs/fcb/src/fcb_test_append.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -7,34 +7,41 @@
 
 #include "fcb_test.h"
 
-ZTEST(fcb_test_with_2sectors_set, test_fcb_append)
+static void test_fcb_append(struct fcb *_fcb)
 {
 	int rc;
-	struct fcb *fcb;
 	struct fcb_entry loc;
 	uint8_t test_data[128];
 	int i;
 	int j;
 	int var_cnt;
 
-	fcb = &test_fcb;
-
 	for (i = 0; i < sizeof(test_data); i++) {
 		for (j = 0; j < i; j++) {
 			test_data[j] = fcb_test_append_data(i, j);
 		}
-		rc = fcb_append(fcb, i, &loc);
+		rc = fcb_append(_fcb, i, &loc);
 		zassert_true(rc == 0, "fcb_append call failure");
-		rc = flash_area_write(fcb->fap, FCB_ENTRY_FA_DATA_OFF(loc),
+		rc = flash_area_write(_fcb->fap, FCB_ENTRY_FA_DATA_OFF(loc),
 				      test_data, i);
 		zassert_true(rc == 0, "flash_area_write call failure");
-		rc = fcb_append_finish(fcb, &loc);
+		rc = fcb_append_finish(_fcb, &loc);
 		zassert_true(rc == 0, "fcb_append_finish call failure");
 	}
 
 	var_cnt = 0;
-	rc = fcb_walk(fcb, 0, fcb_test_data_walk_cb, &var_cnt);
+	rc = fcb_walk(_fcb, 0, fcb_test_data_walk_cb, &var_cnt);
 	zassert_true(rc == 0, "fcb_walk call failure");
 	zassert_true(var_cnt == sizeof(test_data),
 		     "fetched data size not match to wrote data size");
 }
+
+ZTEST(fcb_test_with_2sectors_set, test_fcb_append_2sectors)
+{
+	test_fcb_append(&test_fcb);
+}
+
+ZTEST(fcb_test_crc_disabled, test_fcb_append_crc_disabled)
+{
+	test_fcb_append(&test_fcb_crc_disabled);
+}

tests/subsys/fs/fcb/src/fcb_test_append_too_big.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0

tests/subsys/fs/fcb/src/fcb_test_crc_disabled_after_enabled.c

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2023 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include "fcb_test.h"
+
+static void fcb_pretest_crc_disabled_after_enabled(void)
+{
+	int rc;
+	struct fcb_entry loc;
+	uint8_t test_data[128];
+	int i;
+	int j;
+	int var_cnt;
+
+	for (i = 0; i < sizeof(test_data); i++) {
+		for (j = 0; j < i; j++) {
+			test_data[j] = fcb_test_append_data(i, j);
+		}
+		rc = fcb_append(&test_fcb, i, &loc);
+		zassert_true(rc == 0, "fcb_append call failure");
+		rc = flash_area_write(test_fcb.fap, FCB_ENTRY_FA_DATA_OFF(loc),
+				      test_data, i);
+		zassert_true(rc == 0, "flash_area_write call failure");
+		rc = fcb_append_finish(&test_fcb, &loc);
+		zassert_true(rc == 0, "fcb_append_finish call failure");
+	}
+
+	test_fcb_crc_disabled.f_erase_value = test_fcb.f_erase_value;
+	test_fcb_crc_disabled.f_sector_cnt = test_fcb.f_sector_cnt;
+	test_fcb_crc_disabled.f_sectors = test_fcb.f_sectors;
+
+	rc = fcb_init(TEST_FCB_FLASH_AREA_ID, &test_fcb_crc_disabled);
+	if (rc != 0) {
+		printf("%s rc == %xm, %d\n", __func__, rc, rc);
+		zassert_true(rc == 0, "fbc initialization failure");
+	}
+
+	var_cnt = 0;
+	rc = fcb_walk(&test_fcb_crc_disabled, 0, fcb_test_data_walk_cb, &var_cnt);
+	zassert_true(rc == 0, "fcb_walk call failure");
+	printk("var_cnt: %d", var_cnt);
+	zassert_true(var_cnt == sizeof(test_data),
+		     "fetched data size not match to wrote data size");
+}
+
+
+ZTEST(fcb_test_with_2sectors_set, test_fcb_crc_disabled_after_enabled)
+{
+	fcb_pretest_crc_disabled_after_enabled();
+}

tests/subsys/fs/fcb/src/main.c

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017-2020 Nordic Semiconductor ASA
+ * Copyright (c) 2017-2023 Nordic Semiconductor ASA
  * Copyright (c) 2015 Runtime Inc
  *
  * SPDX-License-Identifier: Apache-2.0
@@ -13,7 +13,9 @@
 #include <zephyr/drivers/flash.h>
 #include <zephyr/device.h>
 
-struct fcb test_fcb;
+struct fcb test_fcb = {0};
+struct fcb test_fcb_crc_disabled = { .f_flags = FCB_FLAGS_CRC_DISABLED };
+
 uint8_t fcb_test_erase_value;
 
 #if defined(CONFIG_SOC_SERIES_STM32H7X)
@@ -108,20 +110,17 @@ int fcb_test_cnt_elems_cb(struct fcb_entry_ctx *entry_ctx, void *arg)
 	return 0;
 }
 
-void fcb_tc_pretest(int sectors)
+void fcb_tc_pretest(int sectors, struct fcb *_fcb)
 {
-	struct fcb *fcb;
 	int rc = 0;
 
 	test_fcb_wipe();
-	fcb = &test_fcb;
+	_fcb->f_erase_value = fcb_test_erase_value;
-	(void)memset(fcb, 0, sizeof(*fcb));
+	_fcb->f_sector_cnt = sectors;
-	fcb->f_erase_value = fcb_test_erase_value;
+	_fcb->f_sectors = test_fcb_sector; /* XXX */
-	fcb->f_sector_cnt = sectors;
-	fcb->f_sectors = test_fcb_sector; /* XXX */
 
 	rc = 0;
-	rc = fcb_init(TEST_FCB_FLASH_AREA_ID, fcb);
+	rc = fcb_init(TEST_FCB_FLASH_AREA_ID, _fcb);
 	if (rc != 0) {
 		printf("%s rc == %xm, %d\n", __func__, rc, rc);
 		zassert_true(rc == 0, "fbc initialization failure");
@@ -130,12 +129,17 @@ void fcb_tc_pretest(int sectors)
 
 static void fcb_pretest_2_sectors(void *data)
 {
-	fcb_tc_pretest(2);
+	fcb_tc_pretest(2, &test_fcb);
 }
 
 static void fcb_pretest_4_sectors(void *data)
 {
-	fcb_tc_pretest(4);
+	fcb_tc_pretest(4, &test_fcb);
+}
+
+static void fcb_pretest_crc_disabled(void *data)
+{
+	fcb_tc_pretest(2, &test_fcb_crc_disabled);
 }
 
 /*
@@ -167,3 +171,5 @@ ZTEST(fcb_test_without_set, test_get_flash_erase_value)
 ZTEST_SUITE(fcb_test_without_set, NULL, NULL, NULL, NULL, NULL);
 ZTEST_SUITE(fcb_test_with_2sectors_set, NULL, NULL, fcb_pretest_2_sectors, NULL, NULL);
 ZTEST_SUITE(fcb_test_with_4sectors_set, NULL, NULL, fcb_pretest_4_sectors, NULL, NULL);
+
+ZTEST_SUITE(fcb_test_crc_disabled, NULL, NULL, fcb_pretest_crc_disabled, NULL, NULL);