xiaoxiang781216 commented on code in PR #17264:
URL: https://github.com/apache/nuttx/pull/17264#discussion_r2513492527


##########
include/nuttx/fs/fs.h:
##########
@@ -625,6 +625,34 @@ int register_driver(FAR const char *path,
                     FAR const struct file_operations *fops, mode_t mode,
                     FAR void *priv);
 
+/****************************************************************************
+ * Name: register_eepromdriver
+ *
+ * Description:
+ *   Register an EEPROM character driver inode into the pseudo file system.
+ *
+ * Input Parameters:
+ *   path - The path to the inode to create
+ *   fops - The file operations structure
+ *   mode - Access privileges
+ *   priv - Private, user data that will be associated with the inode.
+ *   size - EEPROM size in bytes
+ *
+ * Returned Value:
+ *   Zero on success (with the inode point in 'inode'); A negated errno
+ *   value is returned on a failure (all error values returned by
+ *   inode_reserve):
+ *
+ *   EINVAL - 'path' is invalid for this operation
+ *   EEXIST - An inode already exists at 'path'
+ *   ENOMEM - Failed to allocate in-memory resources for the operation
+ *
+ ****************************************************************************/
+
+int register_eepromdriver(FAR const char *path,

Review Comment:
   let's give a general name? since other driver may use this new api to 
initialize file size too.



##########
include/nuttx/fs/fs.h:
##########
@@ -407,7 +407,7 @@ struct inode
   uint16_t          i_flags;    /* Flags for inode */
   union inode_ops_u u;          /* Inode operations */
   ino_t             i_ino;      /* Inode serial number */
-#if defined(CONFIG_PSEUDOFS_FILE) || defined(CONFIG_FS_SHMFS)
+#ifdef CONFIG_FS_INODE_SIZE

Review Comment:
   since any char/block driver could utilize this field now, let's drop 
CONFIG_FS_INODE_SIZE instead?



##########
fs/driver/fs_registerdriver.c:
##########
@@ -67,33 +125,69 @@ int register_driver(FAR const char *path,
                     mode_t mode, FAR void *priv)
 {
   FAR struct inode *node;
-  int ret;
+  int               ret;
 
-  sched_note_mark(NOTE_TAG_DRIVERS, path);
+  ret = lock_populate_inode(path, fops, mode, priv, &node);
 
-  /* Insert a dummy node -- we need to hold the inode semaphore because we
-   * will have a momentarily bad structure.
-   */
+  if (ret == OK)
+    {
+      inode_unlock();
+#ifdef CONFIG_FS_NOTIFY
+      notify_create(path);
+#endif
+    }
 
-  inode_lock();
-  ret = inode_reserve(path, mode, &node);
-  if (ret >= 0)
+  return ret;
+}
+
+#ifdef CONFIG_EEPROM
+
+/****************************************************************************
+ * Name: register_eepromdriver
+ *
+ * Description:
+ *   Register an EEPROM character driver inode into the pseudo file system.
+ *
+ * Input Parameters:
+ *   path - The path to the inode to create
+ *   fops - The file operations structure
+ *   mode - inmode privileges
+ *   priv - Private, user data that will be associated with the inode.
+ *   size - EEPROM size in bytes
+ *
+ * Returned Value:
+ *   Zero on success (with the inode point in 'inode'); A negated errno
+ *   value is returned on a failure (all error values returned by
+ *   inode_reserve):
+ *
+ *   EINVAL - 'path' is invalid for this operation
+ *   EEXIST - An inode already exists at 'path'
+ *   ENOMEM - Failed to allocate in-memory resources for the operation
+ *
+ ****************************************************************************/
+
+int register_eepromdriver(FAR const char *path,

Review Comment:
   why not forward `register_driver(path, fops, mode, priv)` to 
`register_eepromdriver(path, fops, mode, priv, 0)` and remove 
lock_populate_inode?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to