fs: zms: multiple style fixes from previous PR review #80628
+238
−223
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zephyrbot
added
area: File System
area: Documentation Infrastructure
area: Storage
zephyrbot
requested review from
carlescufi,
de-nordic,
gmarull,
kartben,
Laczen and
nashif
de-nordic
previously approved these changes
Oct 30, 2024
tomi-font
reviewed
Nov 4, 2024
tomi-font
reviewed
Nov 5, 2024
This resolves some addressed comments in this PR zephyrproject-rtos#77930 as well as this PR zephyrproject-rtos#80407 Signed-off-by: Riadh Ghaddab <rghaddab@baylibre.com>
rghaddab
force-pushed
the
rghaddab/pr-zms-style-fixup-2
branch
from
f8d7586
to
7262258
Compare
Closed
tomi-font
requested changes
Nov 7, 2024
| * @retval -ERRNO Negative errno code on error | ||
| * | ||
| * @retval 0 on success. | ||
| * @retval -EDEADLK if the detected file system is not ZMS. |
There was a problem hiding this comment.
Using a deadlock error code for that seems quite wrong to me. Can you change that to something else?
| * | ||
| * @retval 0 on success. | ||
| * @retval -EDEADLK if the detected file system is not ZMS. | ||
| * @retval -ENOEXEC if ZMS version is not supported. |
There was a problem hiding this comment.
Maybe EPROTONOSUPPORT (or ENOTSUP) would be more appropriate here?
| * | ||
| * @retval 0 on success. | ||
| * @retval -EDEADLK if the detected file system is not ZMS. | ||
| * @retval -ENOEXEC if ZMS version is not supported. |
There was a problem hiding this comment.
Suggested change
| * @retval -ENOEXEC if ZMS version is not supported. | |
| * @retval -ENOEXEC if the ZMS version is not supported. |
| * @retval -ERRNO Negative errno code on error | ||
| * | ||
| * @retval 0 on success. | ||
| * @retval -EACCES if ZMS is still not initialized. |
There was a problem hiding this comment.
Here do you mean that the passed fs is not mounted?
If so:
Suggested change
| * @retval -EACCES if ZMS is still not initialized. | |
| * @retval -EACCES if `fs` is not mounted. |
Otherwise:
Suggested change
| * @retval -EACCES if ZMS is still not initialized. | |
| * @retval -EACCES if ZMS is not initialized. |
| * | ||
| * @return Number of bytes written. On success, it will be equal to the number of bytes requested | ||
| * to be written or 0. | ||
| * When a rewrite of the same data already stored is attempted, nothing is written to flash, | ||
| * thus 0 is returned. On error, returns negative value of error codes defined in `errno.h`. | ||
| * @retval >=0 on success, number of bytes written. |
There was a problem hiding this comment.
Let's be more explicit?
Suggested change
| * @retval >=0 on success, number of bytes written. | |
| * @retval Number of bytes written (`len` or 0) on success. |
| * @retval -EACCES if ZMS is still not initialized. | ||
| * @retval -ENXIO if there is a device error. | ||
| * @retval -EIO if there is a memory read/write error. | ||
| * @retval -EINVAL if len is in invalid value |
There was a problem hiding this comment.
Suggested change
| * @retval -EINVAL if len is in invalid value | |
| * @retval -EINVAL if `len` is an invalid value. |
or
Suggested change
| * @retval -EINVAL if len is in invalid value | |
| * @retval -EINVAL if `len` is invalid. |
| * @retval -ENXIO if there is a device error. | ||
| * @retval -EIO if there is a memory read/write error. | ||
| * @retval -EINVAL if len is in invalid value | ||
| * @retval -ENOSPC if no space is left on device. |
There was a problem hiding this comment.
Suggested change
| * @retval -ENOSPC if no space is left on device. | |
| * @retval -ENOSPC if no space is left on the device. |
| * | ||
| * @return Number of bytes read. On success, it will be equal to the number of bytes requested | ||
| * to be read or less than that if the stored data has a smaller size than the requested one. | ||
| * On error, returns negative value of error codes defined in `errno.h`. | ||
| * @retval >=0 on success, number of bytes read. |
There was a problem hiding this comment.
Can it actually be 0? (And same for the following functions.)
Suggested change
| * @retval >=0 on success, number of bytes read. | |
| * @retval Number of bytes read (> 0) on success. |
| * @retval >=0 on success, number of bytes read. | ||
| * @retval -EACCES if ZMS is still not initialized. | ||
| * @retval -EIO if there is a memory read/write error. | ||
| * @retval -ENOENT if there is no entry with the given id and history counter. |
There was a problem hiding this comment.
Suggested change
| * @retval -ENOENT if there is no entry with the given id and history counter. | |
| * @retval -ENOENT if there is no entry with the given ID and history counter. |
Suggested change
| * @retval -ENOENT if there is no entry with the given id and history counter. | |
| * @retval -ENOENT if there is no entry with the given `id` and history counter. |
tomi-font
requested changes
Nov 11, 2024
| All the information regarding the effectively available free space in ZMS can be found | ||
| in the documentation. See :ref:`free-space`. | ||
| We recommend choosing a storage partition that can hold double the size of the key-value pairs | ||
| in the documentation. See `Available space for user data <#Available-space-for-user-data-key-value-pairs>`_. |
There was a problem hiding this comment.
This link actually doesn't work (could the documentation build be made to fail in such case @kartben?).
I think it's due to the uppercase. The label doesn't have any.
Suggested change
| in the documentation. See `Available space for user data <#Available-space-for-user-data-key-value-pairs>`_. | |
| in the documentation. See `Available space for user data <available-space-for-user-data-key-value-pairs>`_. |
Ditto for line 282.
tomi-font
reviewed
Nov 11, 2024
| @@ -395,7 +376,7 @@ functionality: :ref:`NVS <nvs_api>` and :ref:`FCB <fcb_api>`. | |||
| Which one to use in your application will depend on your needs and the hardware you are using, | |||
| and this section provides information to help make a choice. | |||
|
|
|||
| - If you are using a non-erasable technology device like RRAM or MRAM, :ref:`ZMS <zms_api>` is definitely the | |||
| - If you are using devices that do not require an erase operation like RRAM or MRAM, :ref:`ZMS <zms_api>` is definitely the | |||
| best fit for your storage subsystem as it is designed to avoid emulating erase operation using | |||
| large block writes for these devices and replaces it with a single write call. | |||
| - For devices with large write_block_size and/or needs a sector size that is different than the | |||
There was a problem hiding this comment.
Suggested change
| - For devices with large write_block_size and/or needs a sector size that is different than the | |
| - For devices that have a large ``write_block_size`` and/or need a sector size that is different than the |
tomi-font
reviewed
Nov 11, 2024
| properties are correct (sector_size, sector_count ...) then calling the zms_init function to | ||
| make the storage ready. | ||
|
|
||
| To mount the filesystem some elements in the zms_fs structure must be initialized. | ||
| To mount the filesystem some elements in the ``zms_fs`` structure must be initialized. |
There was a problem hiding this comment.
Suggested change
| To mount the filesystem some elements in the ``zms_fs`` structure must be initialized. | |
| To mount the filesystem the following elements in the ``zms_fs`` structure must be initialized: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This resolves some addressed comments in this PR #77930 as well as this PR #80407