[wip] bm_storage: updates by lemrey · Pull Request #611 · nrfconnect/sdk-nrf-bm

lemrey · 2026-01-12T09:36:22Z

Support for compiling more than one backend at a time
Add relative addressing and deprecate absolute addressing
Add erase operation in SD backend
Add chunking of writes
Add configurable automatic padding for write operations
Internal refactoring of states
Cleanup alignment checks and docs
Misc fixes
Tests

github-actions · 2026-01-12T09:39:16Z

You can find the documentation preview for this PR here.

eivindj-nordic

Did a first pass.

eivindj-nordic · 2026-01-12T13:57:24Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

 };

-struct bm_storage_sd_state {
+struct {


needs fixing in first commit, not later commit

eivindj-nordic · 2026-01-16T13:46:48Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

 int bm_storage_sd_on_state_evt(enum nrf_sdh_state_evt evt, void *ctx)
 {
-	switch (evt) {
+{	switch (evt) {


Extra {
6cd2050

eivindj-nordic · 2026-01-16T13:47:18Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

 		return 0;
 	}
 }
+}


eivindj-nordic · 2026-01-16T13:49:46Z

subsys/storage/bm_storage/bm_storage.c

 	if (!storage || !config) {
 		return -EFAULT;
 	}
+	if (storage->initialized) {


Add line after } before new if?

eivindj-nordic · 2026-01-16T13:54:59Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

+	enum {
+		OP_NONE,
+		OP_EXECUTING,
+	} operation_state;


I find this a bit confusing together with operation_ongoing. What is used where? Should they be merged?

Update: I think it is clearer if this is squashed with the commit removing operation_ongoing.

eivindj-nordic · 2026-01-16T13:56:25Z

tests/unit/subsys/storage/bm_storage_sd/CMakeLists.txt

@@ -0,0 +1,44 @@
+#
+# Copyright (c) 2025 Nordic Semiconductor ASA


2026 Here and other.

eivindj-nordic · 2026-01-16T14:00:45Z

subsys/storage/bm_storage/sd/bm_storage_sd.c


 const struct bm_storage_info bm_storage_info = {
-	.program_unit = SD_WRITE_BLOCK_SIZE,
+	.program_unit = 4,


Can this value (and erase_unit) be taken from somewhere?

We could use the DT value, but that value is correct only for the hardware. There may be limitations in the backend itself, like in this case, we are limited by the SoftDevice API which writes words. So while 1 byte is correct for the hardware, 4 bytes is the minimum for this backend.

stick with a define so it is clear, the magic number loses all concept

OK, updated

eivindj-nordic · 2026-01-16T14:02:10Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

 	} operation_state;
-	/* Number of times an operation has been retried on timeout. */
-	uint32_t retries;
+	/* Number of times an operation has been retried on timeout */


. is used in comments elsewhere.

ok, reverted

eivindj-nordic · 2026-01-16T14:06:47Z

samples/subsys/storage/bm_storage/src/main.c

 #define STORAGE_B_START STORAGE_A_END
 #define STORAGE_B_END (STORAGE_B_START + BUFFER_BLOCK_SIZE)

+extern const struct bm_storage_api bm_storage_sd_api;


Wonder if there is a better way to do this than with an extern (?).

Yeah, I am not very fond of this solution for this particular use-case. Need to think about it, suggestions welcome.

nordicjm · 2026-01-19T11:23:41Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

 };

-struct bm_storage_sd_state {
+struct {


nordicjm · 2026-01-19T11:24:01Z

subsys/storage/bm_storage/sd/bm_storage_sd.c


-static void on_soc_evt(uint32_t evt, void *ctx);
-static int on_state_evt_change(enum nrf_sdh_state_evt evt, void *ctx);
+void bm_storage_sd_on_soc_evt(uint32_t evt, void *ctx);


This needs to be global so I can invoke it in the unit tests to simulate receiving an SoC event from the SoftDevice.

in that case make it being static or not depend on the unit test Kconfig, normal applications should not have public visibility for this function

I was hoping I could avoid the noise, but have added #ifdefs now

Could do it like e.g. conn params unit tests to avoid the #ifdef https://github.com/nrfconnect/sdk-nrf-bm/blob/main/tests/unit/lib/bluetooth/ble_conn_params/src/unity_test.c#L30

I'll look at that in another PR

nordicjm · 2026-01-19T11:24:54Z

subsys/storage/bm_storage/sd/bm_storage_sd.c

 };

-struct bm_storage_sd_state {
+struct {


needs fixing in first commit, not later commit

nordicjm · 2026-01-19T11:25:30Z

tests/unit/subsys/storage/bm_storage_sd/CMakeLists.txt

+
+find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
+
+project(bm_storage)


bm_storage_sd

nordicjm · 2026-01-19T11:25:50Z

tests/unit/subsys/storage/bm_storage_sd/Kconfig

+
+config BM_STORAGE_BACKEND_SD_QUEUE_SIZE
+	default 4
+config BM_STORAGE_BACKEND_SD_MAX_RETRIES


missing newlines, fix in whole file/whole PR

Hang on, I need one more commit here

nordicjm · 2026-01-19T11:26:53Z

subsys/storage/bm_storage/sd/bm_storage_sd.c


 const struct bm_storage_info bm_storage_info = {
-	.program_unit = SD_WRITE_BLOCK_SIZE,
+	.program_unit = 4,


stick with a define so it is clear, the magic number loses all concept

nordicjm · 2026-01-19T11:27:27Z

include/bm/storage/bm_storage.h

+	int (*uninit)(struct bm_storage *storage);
+	int (*read)(const struct bm_storage *storage, uint32_t src, void *dest, uint32_t len);
+	int (*write)(const struct bm_storage *storage, uint32_t dest, const void *src,
+			   uint32_t len, void *ctx);


misalignment

Rename the variable holding the backend state `type`, to something more meaningful, i.e. `state`. Rename the `bm_storage_sd_state_type` enum to simply `bm_storage_sd_state`. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Wrap loading of items from the ring buffer into a function so that if done from multiple places we don't forget about locking IRQs, and add some semantics to it. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

It is an error to try and initialize the same instance twice, given that each instance has one owner. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

These are checked by the upper layer, since they are common across backends. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

The backends should support unitialization. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Remove the `paused` field and merge that with the queue state. Create a separate operations state to determine when to load the next operation. Remove the `operation_ongoing` flag, which did not indicate whether an operation was ongoing, and it wasn't protecting any shared resource. Add unit test suite. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Add unit tests for bm_storage_rram backend. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

For now the erase operation works by writing the erase value in chunks as big as the erase unit. Initialize both the erase unit and erase value in the bm_storage_info structure. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Add chunking of write operations, similarly to how it was done in the nRF5 SDK. This feature allows to tweak the largest amount of bytes requested to be written by sd_flash_write(). Tweaking this value can help to successfully schedule NVM operations, and optimize the NVM wear. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Create an internal buffer that is filled with the erased value and use that to erase multiple erase units at once, instead of erasing one unit at a time. This speeds up the operation at the cost of little RAM, equal to the _MAX_WRITE_SIZE. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Wrap storing of operations into the queue in a function. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Use a single byte to keep count, to save a few bytes. Add a range to the Kconfig that controls how many times an operation can be retried. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Use a uint8_t rather than a bool, since that's the type accepted as an argument to sd_softdevice_is_enabled(). Rename variable from `sd_enabled` to `softdevice_is_enabled` for clarity. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

The `nvm_info` field is set by `bm_storage` itself upon initialization. Do not check whether it has been set correctly, there is a dedicated field to keep track of whether initialization has happened or not. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

The library should support having multiple instances operating on different backends, for example, the internal NVM on the SoC and an external serial memory. This patch allows choosing the API for an instance during initialization. The bm_storage_backend.h header is removed, instead an API structure is added. The non-volatile memory information is copied by the backend into the storage structure, rather than being just exported by the backend. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

The event dispatch mode must not be determined by looking at whether a context was provided or not. The operation context is fully opaque for the library. Instead, simply check the SoftDevice state. Rename the type from _DISPATCH_TYE to _DISPATCH_MODE for clarity, and verify in the tests that the event mode is reported correctly. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

This error should indicate that we are out of memory to perform the requested operation. Update the SoftDevice backend to return this error instead of EIO when the queue of operations is full. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Add tests for retries and failed operations. Continue processing the queue when an operation fails to be scheduled. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

NVM addresses and operation lengths must be multiples of operation unit. Enforce this check (re-using Zephry macros) in bm_storage and remove redundant checks from the backends. Use EINVAL instead of EFAULT when there is an alignment or out-of-bounds error. Use EFAULT only when a check against a NULL pointer fails. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Update doxygen comment for EPERM return values. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Add two new fields to the configuration structure to set the partition starting address and size. This is aligned with how partitions are defined elsewhere. When `addr` and `size` are set, the API uses relative addressing. Otherwise, it uses absolute addressing. Deprecate the `start_addr` and `end_addr` fields. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Use relative addressing. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Let each backend define its options in a separate file, and put each in a menu for clarity. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Add a flag to automatically pad write operations based on the backend requirements. This simplifies the upper layers that now have an option to avoid adding logic for padding. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

That's now done automatically by the backend. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Add a function to retrieve the NVM info. It would not be correct to do so by peeking into a `bm_storage` structure. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Avoid repeating "Bare Metal Storage" everywhere. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

Minor doxygen updates. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

It is a byte pattern after all. Re-arrange the fields in `bm_storage_info` to optimize for size. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 12, 2026

lemrey force-pushed the storage branch from bf34fbb to d59b406 Compare January 12, 2026 09:58

github-actions bot removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 12, 2026

lemrey force-pushed the storage branch 8 times, most recently from 9698bc0 to 6b25fd0 Compare January 16, 2026 12:33

lemrey marked this pull request as ready for review January 16, 2026 12:52

lemrey requested review from a team and rghaddab as code owners January 16, 2026 12:52

lemrey force-pushed the storage branch from 6b25fd0 to 71a0be9 Compare January 16, 2026 12:52

lemrey requested review from a team and rghaddab as code owners January 16, 2026 12:52

github-actions bot added the doc-required PR must not be merged without tech writer approval. label Jan 16, 2026

eivindj-nordic reviewed Jan 16, 2026

View reviewed changes

lemrey force-pushed the storage branch from 71a0be9 to 9e51785 Compare January 19, 2026 11:18

nordicjm requested changes Jan 19, 2026

View reviewed changes

lemrey force-pushed the storage branch 3 times, most recently from 59444c2 to fcdda1b Compare January 20, 2026 12:33

lemrey added 29 commits February 13, 2026 13:10

storage: bm_storage_sd: wrap loading of queue items into a function

f20225e

Wrap loading of items from the ring buffer into a function so that if done from multiple places we don't forget about locking IRQs, and add some semantics to it. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: treat double initialization as an error

1c08f78

It is an error to try and initialize the same instance twice, given that each instance has one owner. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage_sd: remove redundant initialization checks

6a3599f

These are checked by the upper layer, since they are common across backends. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: provide an unitialization function

43728e8

The backends should support unitialization. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

tests: unit: add tests for bm_storage_rram backend

5346e7b

Add unit tests for bm_storage_rram backend. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: add support for erase operation

3445450

For now the erase operation works by writing the erase value in chunks as big as the erase unit. Initialize both the erase unit and erase value in the bm_storage_info structure. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage_sd: wrap storing operations to queue in a function

7253d14

Wrap storing of operations into the queue in a function. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage_sd: store the number of retries in a single byte

a227040

Use a single byte to keep count, to save a few bytes. Add a range to the Kconfig that controls how many times an operation can be retried. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage_sd: add tests for retries and failed operations

db3ec72

Add tests for retries and failed operations. Continue processing the queue when an operation fails to be scheduled. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: update doxygen comments for EPERM

328ce38

Update doxygen comment for EPERM return values. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

fs: bm_zms: use relative addressing

20ce8e3

Use relative addressing. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: move backend options in separate files

29c9696

Let each backend define its options in a separate file, and put each in a menu for clarity. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

fs: bm_zms: remove padding when writing

188bbb0

That's now done automatically by the backend. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: add function to retrieve nvm info

bce2408

Add a function to retrieve the NVM info. It would not be correct to do so by peeking into a `bm_storage` structure. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: update text for Kconfig options

f0a8f5a

Avoid repeating "Bare Metal Storage" everywhere. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: update doxygen

a82b7c8

Minor doxygen updates. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

storage: bm_storage: let the erase_value be a byte instead of a word

5e306e5

It is a byte pattern after all. Re-arrange the fields in `bm_storage_info` to optimize for size. Signed-off-by: Emanuele Di Santo <emdi@nordicsemi.no>

lemrey force-pushed the storage branch from 448a4e9 to 5e306e5 Compare February 13, 2026 12:13

		@@ -0,0 +1,44 @@
		#
		# Copyright (c) 2025 Nordic Semiconductor ASA


		find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})

		project(bm_storage)

Comments

Conversation

lemrey commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 12, 2026

Uh oh!

eivindj-nordic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

lemrey commented Jan 12, 2026 •

edited

Loading