Skip to content

Conversation

@MirkoCovizzi
Copy link
Contributor

Adds bm_storage library documentation.

@MirkoCovizzi MirkoCovizzi self-assigned this Oct 9, 2025
@MirkoCovizzi MirkoCovizzi requested review from a team as code owners October 9, 2025 14:30
@github-actions github-actions bot added changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval. labels Oct 9, 2025
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

You can find the documentation preview for this PR here.

@MirkoCovizzi MirkoCovizzi force-pushed the storage-docs branch 4 times, most recently from 386377e to d998081 Compare October 9, 2025 15:38
Copy link
Contributor

@eivindj-nordic eivindj-nordic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline I also wonder if the storage init API should take a configuration structure instead of setting parameters in bm_storage strut before calling init.

@MirkoCovizzi MirkoCovizzi force-pushed the storage-docs branch 4 times, most recently from ba80825 to d314076 Compare October 10, 2025 11:57
@MirkoCovizzi MirkoCovizzi removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 10, 2025
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 14, 2025
====

Use the :c:func:`bm_storage_read` function to copy data from NVM into RAM.
The data length must be a multiple of the backend’s program unit and within the configured region.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"must be a multiple of the backend’s program unit"
This leavs me with the question, what is the backend's program unit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the sample we have the comment

Write buffer size must be a multiple of the program unit. 
To support both RRAM (16 bytes) and SoftDevice (4 bytes) backends, that is 16 bytes.

Could probably be added to the documentation.

Copy link

@bjorn-tore-taraldsen bjorn-tore-taraldsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"multiple of the erase unit"
Definition of erase units, where is this defined

@b-gent
Copy link
Contributor

b-gent commented Oct 17, 2025

Adding a commit with a bit of review, to be squashed before merging.

@MirkoCovizzi , can you add the following:

  1. Add information about what we understand by 'program unit' and 'erase unit'.
  2. Add doxygen doc in the header file for the 'len' parameter of the bm_storage_erase function (we mention it explicitly but it is simply not there).

@MirkoCovizzi MirkoCovizzi force-pushed the storage-docs branch 2 times, most recently from 4841ac6 to 1793ab7 Compare November 7, 2025 10:05
Use the :c:func:`bm_storage_write` function to write data to NVM.
Writes are validated against alignment and range, and completion is reported through :c:member:`bm_storage.evt_handler`.

.. note::

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • You don't need repeat of the same note, delete one of them, and there is no NO mention of "Program units" in "Write", but talks about "alignment".


.. note::

The program unit is the smallest block of data that can be written at once.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a reference to where the the program unit is defined.


.. note::

The erase unit is the minimum erasable block in NVM, and the erase length must be a multiple of this size.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for Program unit, reference to where it is defined

@MirkoCovizzi MirkoCovizzi force-pushed the storage-docs branch 3 times, most recently from 1018302 to f4baf9a Compare November 7, 2025 15:01
Adds `bm_storage` library documentation.

Signed-off-by: Mirko Covizzi <[email protected]>
Co-authored-by: Bartosz Gentkowski <[email protected]>
@MirkoCovizzi MirkoCovizzi force-pushed the storage-docs branch 2 times, most recently from 11c29c7 to 339c706 Compare November 7, 2025 15:12
* Adds some missing Doxygen for some input
parameters.

* Make some fields visible by Doxygen.

Signed-off-by: Mirko Covizzi <[email protected]>
@MirkoCovizzi
Copy link
Contributor Author

@bjorn-tore-taraldsen Comments addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants