[nrf fromtree] fs: zms: multiple style fixes from previous PR review

This resolves some addressed comments in this PR zephyrproject-rtos#77930
as well as this PR zephyrproject-rtos#80407

Signed-off-by: Riadh Ghaddab <[email protected]>
(cherry picked from commit 5f7cda5)

Author: rghaddab

doc/services/storage/zms/zms.rst

@@ -980,6 +980,7 @@ INPUT                  = @ZEPHYR_BASE@/doc/_doxygen/mainpage.md \
                          @ZEPHYR_BASE@/subsys/testsuite/include/ \
                          @ZEPHYR_BASE@/subsys/testsuite/ztest/include/ \
                          @ZEPHYR_BASE@/subsys/secure_storage/include/ \
+                         @ZEPHYR_BASE@/subsys/fs/zms/zms_priv.h \
 
 # This tag can be used to specify the character encoding of the source files
 # that Doxygen parses. Internally Doxygen uses the UTF-8 encoding. Doxygen uses

doc/zephyr.doxyfile.in

@@ -80,17 +80,25 @@ struct zms_fs {
  * @brief Mount a ZMS file system onto the device specified in `fs`.
  *
  * @param fs Pointer to the file system.
- * @retval 0 Success
- * @retval -ERRNO Negative errno code on error
+ *
+ * @retval 0 on success.
+ * @retval -ENOTSUP if the detected file system is not ZMS.
+ * @retval -EPROTONOSUPPORT if the ZMS version is not supported.
+ * @retval -EINVAL if any of the flash parameters or the sector layout is invalid.
+ * @retval -ENXIO if there is a device error.
+ * @retval -EIO if there is a memory read/write error.
  */
 int zms_mount(struct zms_fs *fs);
 
 /**
  * @brief Clear the ZMS file system from device.
  *
  * @param fs Pointer to the file system.
- * @retval 0 Success
- * @retval -ERRNO Negative errno code on error
+ *
+ * @retval 0 on success.
+ * @retval -EACCES if `fs` is not mounted.
+ * @retval -ENXIO if there is a device error.
+ * @retval -EIO if there is a memory read/write error.
  */
 int zms_clear(struct zms_fs *fs);
 
@@ -102,65 +110,86 @@ int zms_clear(struct zms_fs *fs);
  * entry and an entry with data of length 0.
  *
  * @param fs Pointer to the file system.
- * @param id ID of the entry to be written
- * @param data Pointer to the data to be written
- * @param len Number of bytes to be written (maximum 64 KiB)
+ * @param id ID of the entry to be written.
+ * @param data Pointer to the data to be written.
+ * @param len Number of bytes to be written (maximum 64 KiB).
  *
  * @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 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 invalid.
+ * @retval -ENOSPC if no space is left on the device.
  */
 ssize_t zms_write(struct zms_fs *fs, uint32_t id, const void *data, size_t len);
 
 /**
  * @brief Delete an entry from the file system
  *
  * @param fs Pointer to the file system.
- * @param id ID of the entry to be deleted
- * @retval 0 Success
- * @retval -ERRNO Negative errno code on error
+ * @param id ID of the entry to be deleted.
+ *
+ * @retval 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.
  */
 int zms_delete(struct zms_fs *fs, uint32_t id);
 
 /**
  * @brief Read an entry from the file system.
  *
  * @param fs Pointer to the file system.
- * @param id ID of the entry to be read
- * @param data Pointer to data buffer
- * @param len Number of bytes to read at most
+ * @param id ID of the entry to be read.
+ * @param data Pointer to data buffer.
+ * @param len Number of bytes to read at most.
  *
  * @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 Number of bytes read (> 0) on success.
+ * @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`.
  */
 ssize_t zms_read(struct zms_fs *fs, uint32_t id, void *data, size_t len);
 
 /**
  * @brief Read a history entry from the file system.
  *
  * @param fs Pointer to the file system.
- * @param id ID of the entry to be read
- * @param data Pointer to data buffer
- * @param len Number of bytes to be read
+ * @param id ID of the entry to be read.
+ * @param data Pointer to data buffer.
+ * @param len Number of bytes to be read.
  * @param cnt History counter: 0: latest entry, 1: one before latest ...
  *
  * @return Number of bytes read. On success, it will be equal to the number of bytes requested
  * to be read. When the return value is larger than the number of bytes requested to read this
  * indicates not all bytes were read, and more data is available. On error, returns negative
  * value of error codes defined in `errno.h`.
+ * @retval Number of bytes read (> 0) on success.
+ * @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.
  */
 ssize_t zms_read_hist(struct zms_fs *fs, uint32_t id, void *data, size_t len, uint32_t cnt);
 
 /**
- * @brief Gets the length of the data that is stored in an entry with a given ID
+ * @brief Gets the length of the data that is stored in an entry with a given `id`
  *
  * @param fs Pointer to the file system.
  * @param id ID of the entry whose data length to retrieve.
  *
  * @return Data length contained in the ATE. On success, it will be equal to the number of bytes
  * in the ATE. On error, returns negative value of error codes defined in `errno.h`.
+ * @retval Length of the entry with the given `id` (> 0) on success.
+ * @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.
  */
 ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id);
 
@@ -173,6 +202,9 @@ ssize_t zms_get_data_length(struct zms_fs *fs, uint32_t id);
  * still be written to the file system.
  * Calculating the free space is a time-consuming operation, especially on SPI flash.
  * On error, returns negative value of error codes defined in `errno.h`.
+ * @retval Number of free bytes (>= 0) on success.
+ * @retval -EACCES if ZMS is still not initialized.
+ * @retval -EIO if there is a memory read/write error.
  */
 ssize_t zms_calc_free_space(struct zms_fs *fs);
 
@@ -181,7 +213,8 @@ ssize_t zms_calc_free_space(struct zms_fs *fs);
  *
  * @param fs Pointer to the file system.
  *
- * @return Number of free bytes.
+ * @retval >=0 Number of free bytes in the currently active sector
+ * @retval -EACCES if ZMS is still not initialized.
  */
 size_t zms_active_sector_free_space(struct zms_fs *fs);
 
@@ -196,7 +229,9 @@ size_t zms_active_sector_free_space(struct zms_fs *fs);
  *
  * @param fs Pointer to the file system.
  *
- * @return 0 on success. On error, returns negative value of error codes defined in `errno.h`.
+ * @retval 0 on success.
+ * @retval -EACCES if ZMS is still not initialized.
+ * @retval -EIO if there is a memory read/write error.
  */
 int zms_sector_use_next(struct zms_fs *fs);
 

include/zephyr/fs/zms.h

@@ -1,3 +1,3 @@
-#SPDX-License-Identifier: Apache-2.0
+# SPDX-License-Identifier: Apache-2.0
 
 zephyr_sources(zms.c)

subsys/fs/zms/CMakeLists.txt

@@ -1,14 +1,16 @@
-#Copyright (c) 2024 BayLibre SAS
+# Copyright (c) 2024 BayLibre SAS
 
-#SPDX-License-Identifier: Apache-2.0
+# SPDX-License-Identifier: Apache-2.0
 
-#Zephyr Memory Storage ZMS
+# Zephyr Memory Storage ZMS
 
 config ZMS
 	bool "Zephyr Memory Storage"
 	select CRC
 	help
-	  Enable support of Zephyr Memory Storage.
+	  Enable Zephyr Memory Storage, which is a key-value storage system designed to work with
+	  all types of non-volatile storage technologies.
+	  It supports classical on-chip NOR flash as well as new technologies like RRAM and MRAM.
 
 if ZMS
 
@@ -20,28 +22,25 @@ config ZMS_LOOKUP_CACHE
 	  table entry (ATE) for all ZMS IDs that fall into that cache position.
 
 config ZMS_LOOKUP_CACHE_SIZE
-	int "ZMS Storage lookup cache size"
+	int "ZMS lookup cache size"
 	default 128
 	range 1 65536
 	depends on ZMS_LOOKUP_CACHE
 	help
-	  Number of entries in ZMS lookup cache.
-	  It is recommended that it should be a power of 2.
-	  Every additional entry in cache will add 8 bytes in RAM
+	  Number of entries in the ZMS lookup cache.
+	  Every additional entry in cache will use 8 bytes of RAM.
 
 config ZMS_DATA_CRC
-	bool "ZMS DATA CRC"
-	help
-	  Enables DATA CRC
+	bool "ZMS data CRC"
 
 config ZMS_CUSTOMIZE_BLOCK_SIZE
 	bool "Customize the size of the buffer used internally for reads and writes"
 	help
 	  ZMS uses an internal buffer to read/write and compare stored data.
 	  Increasing the size of this buffer should be done carefully in order to not
 	  overflow the stack.
-	  Increasing this buffer means as well that ZMS could work with storage devices
-	  that have larger write-block-size which decreases ZMS performance
+	  Increasing it makes ZMS able to work with storage devices
+	  that have a larger `write-block-size` (which decreases the performance of ZMS).
 
 config ZMS_CUSTOM_BLOCK_SIZE
 	int "ZMS internal buffer size"

subsys/fs/zms/Kconfig

@@ -1101,7 +1101,7 @@ static int zms_init(struct zms_fs *fs)
 				/* Let's check that we support this ZMS version */
 				if (ZMS_GET_VERSION(empty_ate.metadata) != ZMS_DEFAULT_VERSION) {
 					LOG_ERR("ZMS Version is not supported");
-					rc = -ENOEXEC;
+					rc = -EPROTONOSUPPORT;
 					goto end;
 				}
 			}
@@ -1125,7 +1125,7 @@ static int zms_init(struct zms_fs *fs)
 	}
 	/* all sectors are closed, and zms magic number not found. This is not a zms fs */
 	if ((closed_sectors == fs->sector_count) && !zms_magic_exist) {
-		rc = -EDEADLK;
+		rc = -ENOTSUP;
 		goto end;
 	}
 	/* TODO: add a recovery mechanism here if the ZMS magic number exist but all
@@ -1157,7 +1157,7 @@ static int zms_init(struct zms_fs *fs)
 				/* Let's check the version */
 				if (ZMS_GET_VERSION(empty_ate.metadata) != ZMS_DEFAULT_VERSION) {
 					LOG_ERR("ZMS Version is not supported");
-					rc = -ENOEXEC;
+					rc = -EPROTONOSUPPORT;
 					goto end;
 				}
 			}

subsys/fs/zms/zms.c

@@ -8,15 +8,11 @@
 #ifndef __ZMS_PRIV_H_
 #define __ZMS_PRIV_H_
 
-#ifdef __cplusplus
-extern "C" {
-#endif
-
 /*
- * MASKS AND SHIFT FOR ADDRESSES
- * an address in zms is an uint64_t where:
- *   high 4 bytes represent the sector number
- *   low 4 bytes represent the offset in a sector
+ * MASKS AND SHIFT FOR ADDRESSES.
+ * An address in zms is an uint64_t where:
+ * - high 4 bytes represent the sector number
+ * - low 4 bytes represent the offset in a sector
  */
 #define ADDR_SECT_MASK   GENMASK64(63, 32)
 #define ADDR_SECT_SHIFT  32
@@ -44,34 +40,40 @@ extern "C" {
 #define ZMS_INVALID_SECTOR_NUM -1
 #define ZMS_DATA_IN_ATE_SIZE   8
 
+/**
+ * @ingroup zms_data_structures
+ * ZMS Allocation Table Entry (ATE) structure
+ */
 struct zms_ate {
-	uint8_t crc8;      /* crc8 check of the entry */
-	uint8_t cycle_cnt; /* cycle counter for non erasable devices */
-	uint16_t len;      /* data len within sector */
-	uint32_t id;       /* data id */
+	/** crc8 check of the entry */
+	uint8_t crc8;
+	/** cycle counter for non erasable devices */
+	uint8_t cycle_cnt;
+	/** data len within sector */
+	uint16_t len;
+	/** data id */
+	uint32_t id;
 	union {
-		uint8_t data[8]; /* used to store small size data */
+		/** data field used to store small sized data */
+		uint8_t data[8];
 		struct {
-			uint32_t offset; /* data offset within sector */
+			/** data offset within sector */
+			uint32_t offset;
 			union {
-				uint32_t data_crc; /*
-						    * crc for data: The data CRC is checked only
-						    * when the whole data of the element is read.
-						    * The data CRC is not checked for a partial
-						    * read, as it is computed for the complete
-						    * set of data.
-						    */
-				uint32_t metadata; /*
-						    * Used to store metadata information
-						    * such as storage version.
-						    */
+				/**
+				 * crc for data: The data CRC is checked only when the whole data
+				 * of the element is read.
+				 * The data CRC is not checked for a partial read, as it is computed
+				 * for the complete set of data.
+				 */
+				uint32_t data_crc;
+				/**
+				 * Used to store metadata information such as storage version.
+				 */
+				uint32_t metadata;
 			};
 		};
 	};
 } __packed;
 
-#ifdef __cplusplus
-}
-#endif
-
 #endif /* __ZMS_PRIV_H_ */