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


##########
boards/xtensa/esp32/common/src/esp32_board_spiflash.c:
##########
@@ -405,6 +405,17 @@ static int init_storage_partition(void)
       return ret;
     }
 
+#elif defined(CONFIG_MTD_NVBLK)
+
+  ret = nvblk_initialize(0, mtd, CONFIG_MTD_NVBLK_DEFAULT_LBS,

Review Comment:
   it's better to call register_mtddriver and hook nvblk in the common place:
   https://github.com/apache/nuttx/blob/master/fs/driver/fs_mtdproxy.c#L168
   so, we can wrap mtd with either ftl/nvblk/dhara automatically.



##########
drivers/mtd/nvblk.c:
##########
@@ -0,0 +1,599 @@
+/****************************************************************************
+ * drivers/mtd/nvblk.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include <nuttx/nuttx.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#include <nuttx/lib/lib.h>
+
+#include <nvblk/nvblk.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define NVBLK_MIN_LBS 128 /* Minimal logical block size */
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct nvblk_dev_s
+{
+  struct nvb_config     cfg;         /* nvblk configuration */
+  struct nvb_info       info;        /* nvblk data */
+  mutex_t               lock;
+
+  FAR struct mtd_dev_s *mtd;         /* Contained MTD interface */
+  struct mtd_geometry_s geo;         /* Device geometry */
+  uint16_t              refs;        /* Number of references */
+  uint8_t               log2_bpiob;  /* (logical) blocks per IO block */
+  uint8_t               log2_ppb;    /* pages per (logical) block */
+  bool                  unlinked;    /* The driver has been unlinked */
+
+  /* Two pagesize buffer first is for working temp buffer
+   * second is for journel use
+   */
+
+  FAR uint8_t *pagebuf;
+};
+
+typedef struct nvblk_dev_s nvblk_dev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int     nvblk_open(FAR struct inode *inode);
+static int     nvblk_close(FAR struct inode *inode);
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors);
+static ssize_t nvblk_write(FAR struct inode *inode,
+                           FAR const unsigned char *buffer,
+                           blkcnt_t start_sector,
+                           unsigned int nsectors);
+static int     nvblk_geometry(FAR struct inode *inode,
+                              FAR struct geometry *geometry);
+static int     nvblk_ioctl(FAR struct inode *inode,
+                           int cmd,
+                           unsigned long arg);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int     nvblk_unlink(FAR struct inode *inode);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct block_operations g_nvblk_bops =
+{
+  nvblk_open,     /* open     */
+  nvblk_close,    /* close    */
+  nvblk_read,     /* read     */
+  nvblk_write,    /* write    */
+  nvblk_geometry, /* geometry */
+  nvblk_ioctl     /* ioctl    */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  , nvblk_unlink  /* unlink   */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int nvblk_convert_result(int err)
+{
+  switch (err)
+    {
+      case 0:
+        return 0;
+      case -NVB_ENOENT:
+        return -ENOENT;
+      case -NVB_EINVAL:
+        return -EINVAL;
+      case -NVB_EROFS:
+        return -EROFS;
+      case -NVB_EAGAIN:
+        return -EAGAIN;
+      case -NVB_ENOSPC:
+        return -ENOSPC;
+      default:
+        return -EFAULT;
+    }
+}
+
+/****************************************************************************
+ * Name: nvblk_open
+ *
+ * Description: Open the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_open(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs++;
+  nxmutex_unlock(&dev->lock);
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_close
+ *
+ * Description: close the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_close(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs--;
+  nxmutex_unlock(&dev->lock);
+
+  if (dev->refs == 0 && dev->unlinked)
+    {
+      nxmutex_destroy(&dev->lock);
+      kmm_free(dev->pagebuf);
+      kmm_free(dev);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_read
+ *
+ * Description:  Read the specified number of sectors
+ *
+ ****************************************************************************/
+
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors)
+{
+  FAR nvblk_dev_t *dev;
+  uint16_t bstart;
+  uint16_t bcnt;
+  int ret = 0;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  bstart = start_sector << dev->log2_bpiob;
+  bcnt = nsectors << dev->log2_bpiob;
+
+  for (int n = 0; n < bcnt; n++)

Review Comment:
   move n definition to begin of function and change to uint16_t



##########
include/nuttx/mtd/mtd.h:
##########
@@ -820,6 +820,53 @@ int dhara_initialize_by_path(FAR const char *path,
                              FAR struct mtd_dev_s *mtd);
 #endif
 
+/****************************************************************************
+ * Name: nvblk_initialize
+ *
+ * Description:
+ *   Initialize to provide a block driver wrapper around an MTD interface
+ *
+ * Input Parameters:
+ *   minor - The minor device number.  The MTD block device will be
+ *           registered as as /dev/mtdblockN where N is the minor number.
+ *   mtd   - The MTD device that supports the FLASH interface.
+ *   lbs  - The logical blocksize (size of the nvblk blocks).
+ *   iobs - The input output blocksize (multiple of lbs).
+ *   speb - The number of spare erase blocks.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_MTD_NVBLK
+int nvblk_initialize(int minor,
+                     FAR struct mtd_dev_s *mtd,
+                     uint32_t lbs,
+                     uint32_t iobs,
+                     uint32_t speb);
+#endif
+
+/****************************************************************************
+ * Name: nvblk_initialize_by_path
+ *
+ * Description:
+ *   Initialize to provide a block driver wrapper around an MTD interface
+ *
+ * Input Parameters:
+ *   path - The block device path.
+ *   mtd  - The MTD device that supports the FLASH interface.
+ *   lbs  - The logical blocksize (size of the nvblk blocks).
+ *   iobs - The input output blocksize (multiple of lbs).
+ *   speb - The number of spare erase blocks.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_MTD_NVBLK
+int nvblk_initialize_by_path(FAR const char *path,

Review Comment:
   ```suggestion
   int nvblk_initialize(FAR const char *path,
   ```
   and remove line 840, like https://github.com/apache/nuttx/pull/16793



##########
drivers/mtd/nvblk.c:
##########
@@ -0,0 +1,599 @@
+/****************************************************************************
+ * drivers/mtd/nvblk.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include <nuttx/nuttx.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#include <nuttx/lib/lib.h>
+
+#include <nvblk/nvblk.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define NVBLK_MIN_LBS 128 /* Minimal logical block size */
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct nvblk_dev_s
+{
+  struct nvb_config     cfg;         /* nvblk configuration */
+  struct nvb_info       info;        /* nvblk data */
+  mutex_t               lock;
+
+  FAR struct mtd_dev_s *mtd;         /* Contained MTD interface */
+  struct mtd_geometry_s geo;         /* Device geometry */
+  uint16_t              refs;        /* Number of references */
+  uint8_t               log2_bpiob;  /* (logical) blocks per IO block */
+  uint8_t               log2_ppb;    /* pages per (logical) block */
+  bool                  unlinked;    /* The driver has been unlinked */
+
+  /* Two pagesize buffer first is for working temp buffer
+   * second is for journel use
+   */
+
+  FAR uint8_t *pagebuf;
+};
+
+typedef struct nvblk_dev_s nvblk_dev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int     nvblk_open(FAR struct inode *inode);
+static int     nvblk_close(FAR struct inode *inode);
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors);
+static ssize_t nvblk_write(FAR struct inode *inode,
+                           FAR const unsigned char *buffer,
+                           blkcnt_t start_sector,
+                           unsigned int nsectors);
+static int     nvblk_geometry(FAR struct inode *inode,
+                              FAR struct geometry *geometry);
+static int     nvblk_ioctl(FAR struct inode *inode,
+                           int cmd,
+                           unsigned long arg);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int     nvblk_unlink(FAR struct inode *inode);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct block_operations g_nvblk_bops =
+{
+  nvblk_open,     /* open     */
+  nvblk_close,    /* close    */
+  nvblk_read,     /* read     */
+  nvblk_write,    /* write    */
+  nvblk_geometry, /* geometry */
+  nvblk_ioctl     /* ioctl    */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  , nvblk_unlink  /* unlink   */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int nvblk_convert_result(int err)
+{
+  switch (err)
+    {
+      case 0:
+        return 0;
+      case -NVB_ENOENT:
+        return -ENOENT;
+      case -NVB_EINVAL:
+        return -EINVAL;
+      case -NVB_EROFS:
+        return -EROFS;
+      case -NVB_EAGAIN:
+        return -EAGAIN;
+      case -NVB_ENOSPC:
+        return -ENOSPC;
+      default:
+        return -EFAULT;
+    }
+}
+
+/****************************************************************************
+ * Name: nvblk_open
+ *
+ * Description: Open the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_open(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs++;
+  nxmutex_unlock(&dev->lock);
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_close
+ *
+ * Description: close the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_close(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs--;
+  nxmutex_unlock(&dev->lock);
+
+  if (dev->refs == 0 && dev->unlinked)
+    {
+      nxmutex_destroy(&dev->lock);
+      kmm_free(dev->pagebuf);
+      kmm_free(dev);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_read
+ *
+ * Description:  Read the specified number of sectors
+ *
+ ****************************************************************************/
+
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors)
+{
+  FAR nvblk_dev_t *dev;
+  uint16_t bstart;
+  uint16_t bcnt;
+  int ret = 0;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  bstart = start_sector << dev->log2_bpiob;
+  bcnt = nsectors << dev->log2_bpiob;
+
+  for (int n = 0; n < bcnt; n++)
+    {
+      ret = nvb_read(&dev->info, buffer, bstart, 1);
+      if (ret == -NVB_ENOENT)
+        {
+          memset(buffer, 0xff, (1 << dev->cfg.log2_bs));
+          ret = 0;
+        }
+
+      if (ret < 0)
+        {
+          break;
+        }
+
+      buffer += (1 << dev->cfg.log2_bs);
+      bstart++;
+    }
+
+  if (ret < 0)
+    {
+      ret = nvblk_convert_result(ret);
+      ferr("Read startblock %lld failed nsectors %zd err: %d\n",
+           (long long)start_sector, nsectors, ret);
+    }
+
+  nxmutex_unlock(&dev->lock);
+  return ret < 0 ? ret : nsectors;
+}
+
+/****************************************************************************
+ * Name: nvblk_write
+ *
+ * Description: Write the specified number of sectors
+ *
+ ****************************************************************************/
+
+static ssize_t nvblk_write(FAR struct inode *inode,
+                           FAR const unsigned char *buffer,
+                           blkcnt_t start_sector,
+                           unsigned int nsectors)
+{
+  FAR nvblk_dev_t *dev;
+  uint16_t bstart;
+  uint16_t bcnt;
+  int ret = 0;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  bstart = start_sector << dev->log2_bpiob;
+  bcnt = nsectors << dev->log2_bpiob;
+  ret = nvblk_convert_result(nvb_write(&dev->info, buffer, bstart, bcnt));
+  if (ret < 0)
+    {
+      ferr("Write startblock %lld failed nsectors %zd err: %d\n",
+           (long long)start_sector, nsectors, ret);
+    }
+
+  nxmutex_unlock(&dev->lock);
+  return ret < 0 ? ret : nsectors;
+}
+
+/****************************************************************************
+ * Name: nvblk_geometry
+ *
+ * Description: Return device geometry
+ *
+ ****************************************************************************/
+
+static int nvblk_geometry(FAR struct inode *inode,
+                          FAR struct geometry *geometry)
+{
+  FAR nvblk_dev_t *dev;
+  uint32_t blkcnt;
+  int ret = -EINVAL;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+
+  if (!geometry)
+    {
+      goto end;
+    }
+
+  nxmutex_lock(&dev->lock);
+  ret = nvb_ioctl(&dev->info, NVB_CMD_GET_BLK_COUNT, &blkcnt);
+
+  if (ret < 0)
+    {
+      ret = nvblk_convert_result(ret);
+      goto end;
+    }
+
+  geometry->geo_available    = true;
+  geometry->geo_mediachanged = false;
+  geometry->geo_writeenabled = true;
+  geometry->geo_nsectors     = blkcnt >> dev->log2_bpiob;
+  geometry->geo_sectorsize   = (1 << (dev->log2_bpiob + dev->cfg.log2_bs));
+  strcpy(geometry->geo_model, dev->geo.model);

Review Comment:
   strlcpy



##########
drivers/mtd/nvblk.c:
##########
@@ -0,0 +1,599 @@
+/****************************************************************************
+ * drivers/mtd/nvblk.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include <nuttx/nuttx.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#include <nuttx/lib/lib.h>
+
+#include <nvblk/nvblk.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define NVBLK_MIN_LBS 128 /* Minimal logical block size */
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct nvblk_dev_s
+{
+  struct nvb_config     cfg;         /* nvblk configuration */
+  struct nvb_info       info;        /* nvblk data */
+  mutex_t               lock;
+
+  FAR struct mtd_dev_s *mtd;         /* Contained MTD interface */
+  struct mtd_geometry_s geo;         /* Device geometry */
+  uint16_t              refs;        /* Number of references */
+  uint8_t               log2_bpiob;  /* (logical) blocks per IO block */
+  uint8_t               log2_ppb;    /* pages per (logical) block */
+  bool                  unlinked;    /* The driver has been unlinked */
+
+  /* Two pagesize buffer first is for working temp buffer
+   * second is for journel use
+   */
+
+  FAR uint8_t *pagebuf;
+};
+
+typedef struct nvblk_dev_s nvblk_dev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int     nvblk_open(FAR struct inode *inode);
+static int     nvblk_close(FAR struct inode *inode);
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors);
+static ssize_t nvblk_write(FAR struct inode *inode,
+                           FAR const unsigned char *buffer,
+                           blkcnt_t start_sector,
+                           unsigned int nsectors);
+static int     nvblk_geometry(FAR struct inode *inode,
+                              FAR struct geometry *geometry);
+static int     nvblk_ioctl(FAR struct inode *inode,
+                           int cmd,
+                           unsigned long arg);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int     nvblk_unlink(FAR struct inode *inode);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct block_operations g_nvblk_bops =
+{
+  nvblk_open,     /* open     */
+  nvblk_close,    /* close    */
+  nvblk_read,     /* read     */
+  nvblk_write,    /* write    */
+  nvblk_geometry, /* geometry */
+  nvblk_ioctl     /* ioctl    */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  , nvblk_unlink  /* unlink   */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int nvblk_convert_result(int err)
+{
+  switch (err)
+    {
+      case 0:
+        return 0;
+      case -NVB_ENOENT:
+        return -ENOENT;
+      case -NVB_EINVAL:
+        return -EINVAL;
+      case -NVB_EROFS:
+        return -EROFS;
+      case -NVB_EAGAIN:
+        return -EAGAIN;
+      case -NVB_ENOSPC:
+        return -ENOSPC;
+      default:
+        return -EFAULT;
+    }
+}
+
+/****************************************************************************
+ * Name: nvblk_open
+ *
+ * Description: Open the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_open(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs++;
+  nxmutex_unlock(&dev->lock);
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_close
+ *
+ * Description: close the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_close(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs--;
+  nxmutex_unlock(&dev->lock);
+
+  if (dev->refs == 0 && dev->unlinked)
+    {
+      nxmutex_destroy(&dev->lock);
+      kmm_free(dev->pagebuf);
+      kmm_free(dev);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_read
+ *
+ * Description:  Read the specified number of sectors
+ *
+ ****************************************************************************/
+
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors)
+{
+  FAR nvblk_dev_t *dev;
+  uint16_t bstart;
+  uint16_t bcnt;
+  int ret = 0;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  bstart = start_sector << dev->log2_bpiob;
+  bcnt = nsectors << dev->log2_bpiob;
+
+  for (int n = 0; n < bcnt; n++)
+    {
+      ret = nvb_read(&dev->info, buffer, bstart, 1);

Review Comment:
   why not read in one call



##########
drivers/mtd/nvblk.c:
##########
@@ -0,0 +1,599 @@
+/****************************************************************************
+ * drivers/mtd/nvblk.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include <nuttx/nuttx.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#include <nuttx/lib/lib.h>
+
+#include <nvblk/nvblk.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define NVBLK_MIN_LBS 128 /* Minimal logical block size */
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct nvblk_dev_s
+{
+  struct nvb_config     cfg;         /* nvblk configuration */
+  struct nvb_info       info;        /* nvblk data */
+  mutex_t               lock;
+
+  FAR struct mtd_dev_s *mtd;         /* Contained MTD interface */
+  struct mtd_geometry_s geo;         /* Device geometry */
+  uint16_t              refs;        /* Number of references */
+  uint8_t               log2_bpiob;  /* (logical) blocks per IO block */
+  uint8_t               log2_ppb;    /* pages per (logical) block */
+  bool                  unlinked;    /* The driver has been unlinked */
+
+  /* Two pagesize buffer first is for working temp buffer
+   * second is for journel use
+   */
+
+  FAR uint8_t *pagebuf;
+};
+
+typedef struct nvblk_dev_s nvblk_dev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int     nvblk_open(FAR struct inode *inode);
+static int     nvblk_close(FAR struct inode *inode);
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors);
+static ssize_t nvblk_write(FAR struct inode *inode,
+                           FAR const unsigned char *buffer,
+                           blkcnt_t start_sector,
+                           unsigned int nsectors);
+static int     nvblk_geometry(FAR struct inode *inode,
+                              FAR struct geometry *geometry);
+static int     nvblk_ioctl(FAR struct inode *inode,
+                           int cmd,
+                           unsigned long arg);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+static int     nvblk_unlink(FAR struct inode *inode);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct block_operations g_nvblk_bops =
+{
+  nvblk_open,     /* open     */
+  nvblk_close,    /* close    */
+  nvblk_read,     /* read     */
+  nvblk_write,    /* write    */
+  nvblk_geometry, /* geometry */
+  nvblk_ioctl     /* ioctl    */
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+  , nvblk_unlink  /* unlink   */
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int nvblk_convert_result(int err)
+{
+  switch (err)
+    {
+      case 0:
+        return 0;
+      case -NVB_ENOENT:
+        return -ENOENT;
+      case -NVB_EINVAL:
+        return -EINVAL;
+      case -NVB_EROFS:
+        return -EROFS;
+      case -NVB_EAGAIN:
+        return -EAGAIN;
+      case -NVB_ENOSPC:
+        return -ENOSPC;
+      default:
+        return -EFAULT;
+    }
+}
+
+/****************************************************************************
+ * Name: nvblk_open
+ *
+ * Description: Open the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_open(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs++;
+  nxmutex_unlock(&dev->lock);
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_close
+ *
+ * Description: close the block device
+ *
+ ****************************************************************************/
+
+static int nvblk_close(FAR struct inode *inode)
+{
+  FAR nvblk_dev_t *dev;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  dev->refs--;
+  nxmutex_unlock(&dev->lock);
+
+  if (dev->refs == 0 && dev->unlinked)
+    {
+      nxmutex_destroy(&dev->lock);
+      kmm_free(dev->pagebuf);
+      kmm_free(dev);
+    }
+
+  return 0;
+}
+
+/****************************************************************************
+ * Name: nvblk_read
+ *
+ * Description:  Read the specified number of sectors
+ *
+ ****************************************************************************/
+
+static ssize_t nvblk_read(FAR struct inode *inode,
+                          FAR unsigned char *buffer,
+                          blkcnt_t start_sector,
+                          unsigned int nsectors)
+{
+  FAR nvblk_dev_t *dev;
+  uint16_t bstart;
+  uint16_t bcnt;
+  int ret = 0;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  bstart = start_sector << dev->log2_bpiob;
+  bcnt = nsectors << dev->log2_bpiob;
+
+  for (int n = 0; n < bcnt; n++)
+    {
+      ret = nvb_read(&dev->info, buffer, bstart, 1);
+      if (ret == -NVB_ENOENT)
+        {
+          memset(buffer, 0xff, (1 << dev->cfg.log2_bs));
+          ret = 0;
+        }
+
+      if (ret < 0)
+        {
+          break;
+        }
+
+      buffer += (1 << dev->cfg.log2_bs);
+      bstart++;
+    }
+
+  if (ret < 0)
+    {
+      ret = nvblk_convert_result(ret);
+      ferr("Read startblock %lld failed nsectors %zd err: %d\n",
+           (long long)start_sector, nsectors, ret);
+    }
+
+  nxmutex_unlock(&dev->lock);
+  return ret < 0 ? ret : nsectors;
+}
+
+/****************************************************************************
+ * Name: nvblk_write
+ *
+ * Description: Write the specified number of sectors
+ *
+ ****************************************************************************/
+
+static ssize_t nvblk_write(FAR struct inode *inode,
+                           FAR const unsigned char *buffer,
+                           blkcnt_t start_sector,
+                           unsigned int nsectors)
+{
+  FAR nvblk_dev_t *dev;
+  uint16_t bstart;
+  uint16_t bcnt;
+  int ret = 0;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+  nxmutex_lock(&dev->lock);
+  bstart = start_sector << dev->log2_bpiob;
+  bcnt = nsectors << dev->log2_bpiob;
+  ret = nvblk_convert_result(nvb_write(&dev->info, buffer, bstart, bcnt));
+  if (ret < 0)
+    {
+      ferr("Write startblock %lld failed nsectors %zd err: %d\n",
+           (long long)start_sector, nsectors, ret);
+    }
+
+  nxmutex_unlock(&dev->lock);
+  return ret < 0 ? ret : nsectors;
+}
+
+/****************************************************************************
+ * Name: nvblk_geometry
+ *
+ * Description: Return device geometry
+ *
+ ****************************************************************************/
+
+static int nvblk_geometry(FAR struct inode *inode,
+                          FAR struct geometry *geometry)
+{
+  FAR nvblk_dev_t *dev;
+  uint32_t blkcnt;
+  int ret = -EINVAL;
+
+  DEBUGASSERT(inode->i_private);
+  dev = inode->i_private;
+
+  if (!geometry)
+    {
+      goto end;

Review Comment:
   return directly and remove end label



-- 
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: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to