xiaoxiang781216 commented on code in PR #8573: URL: https://github.com/apache/nuttx/pull/8573#discussion_r1111031560
########## include/nuttx/fs/loopmtd.h: ########## @@ -0,0 +1,107 @@ +/**************************************************************************** + * include/nuttx/fs/loopmtd.h + * + * 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. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FS_FILEMTD_H +#define __INCLUDE_NUTTX_FS_FILEMTD_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <sys/types.h> +#include <sys/ioctl.h> +#include <stdint.h> +#include <stdbool.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +/* Loop device IOCTL commands */ + +/* Command: MTD_LOOPIOC_SETUP + * Description: Setup the loop device + * Argument: A pointer to a read-only instance of struct losetup_s. + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +/* Command: MTD_LOOPIOC_TEARDOWN + * Description: Teardown a loop device previously setup vis LOOPIOC_SETUP + * Argument: A read-able pointer to the path of the device to be + * torn down + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +#define MTD_LOOPIOC_SETUP _LOOPIOC(0x0001) +#define MTD_LOOPIOC_TEARDOWN _LOOPIOC(0x0002) + +#endif + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +/* This is the structure referred to in the argument to the LOOPIOC_SETUP + * IOCTL command. + */ + +struct mtd_losetup_s +{ + FAR const char *devname; /* The loop mtd device to be created */ + FAR const char *filename; /* The file or character device to use */ + int erasesize; /* The erase size to use on the file */ + int sectsize; /* The sector / page size of the file */ Review Comment: change both int to size_t ########## include/nuttx/fs/loopmtd.h: ########## @@ -0,0 +1,107 @@ +/**************************************************************************** + * include/nuttx/fs/loopmtd.h + * + * 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. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FS_FILEMTD_H +#define __INCLUDE_NUTTX_FS_FILEMTD_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <sys/types.h> +#include <sys/ioctl.h> +#include <stdint.h> +#include <stdbool.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +/* Loop device IOCTL commands */ + +/* Command: MTD_LOOPIOC_SETUP + * Description: Setup the loop device + * Argument: A pointer to a read-only instance of struct losetup_s. + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +/* Command: MTD_LOOPIOC_TEARDOWN + * Description: Teardown a loop device previously setup vis LOOPIOC_SETUP + * Argument: A read-able pointer to the path of the device to be + * torn down + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +#define MTD_LOOPIOC_SETUP _LOOPIOC(0x0001) +#define MTD_LOOPIOC_TEARDOWN _LOOPIOC(0x0002) + +#endif + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP Review Comment: CONFIG_MTD_DEV_LOOP->CONFIG_MTD_LOOP ########## drivers/drivers_initialize.c: ########## @@ -189,4 +190,7 @@ void drivers_initialize(void) #ifdef CONFIG_SMART_DEV_LOOP smart_loop_register_driver(); #endif +#ifdef CONFIG_MTD_DEV_LOOP + mtd_loop_register_driver(); Review Comment: merge to previous patch ########## include/nuttx/fs/loopmtd.h: ########## @@ -0,0 +1,107 @@ +/**************************************************************************** + * include/nuttx/fs/loopmtd.h + * + * 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. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FS_FILEMTD_H +#define __INCLUDE_NUTTX_FS_FILEMTD_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <sys/types.h> +#include <sys/ioctl.h> +#include <stdint.h> +#include <stdbool.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +/* Loop device IOCTL commands */ + +/* Command: MTD_LOOPIOC_SETUP + * Description: Setup the loop device + * Argument: A pointer to a read-only instance of struct losetup_s. + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +/* Command: MTD_LOOPIOC_TEARDOWN + * Description: Teardown a loop device previously setup vis LOOPIOC_SETUP + * Argument: A read-able pointer to the path of the device to be + * torn down + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) Review Comment: which Kconfig define MTD_DEV_LOOP? ########## drivers/mtd/filemtd.c: ########## @@ -708,3 +735,201 @@ bool filemtd_isfilemtd(FAR struct mtd_dev_s *dev) return (priv->mtd.erase == filemtd_erase); } + +/**************************************************************************** + * Name: mtd_loop_register_driver + * + * Description: + * Registers MTD Loop Driver + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +int mtd_loop_register_driver(void) +{ + return register_driver("/dev/loopmtd", &g_fops, 0666, NULL); +} +#endif + +/**************************************************************************** + * Name: mtd_loop_setup + * + * Description: Dynamically setups up a FILEMTD enabled loop device that + * is backed by a file. The resulting loop device is a + * MTD type block device vs. a generic block device. + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_setup(FAR const char *devname, FAR const char *filename, + int sectsize, int erasesize, off_t offset) +{ + FAR struct mtd_dev_s *mtd; + int ret; + + mtd = filemtd_initialize(filename, offset, sectsize, erasesize); + if (mtd == NULL) + { + return -ENOENT; + } + + ret = register_mtddriver(devname, mtd, 0755, NULL); + Review Comment: remove the blank line ########## include/nuttx/fs/loopmtd.h: ########## @@ -0,0 +1,107 @@ +/**************************************************************************** + * include/nuttx/fs/loopmtd.h + * + * 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. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FS_FILEMTD_H +#define __INCLUDE_NUTTX_FS_FILEMTD_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <sys/types.h> +#include <sys/ioctl.h> +#include <stdint.h> +#include <stdbool.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +/* Loop device IOCTL commands */ + +/* Command: MTD_LOOPIOC_SETUP + * Description: Setup the loop device + * Argument: A pointer to a read-only instance of struct losetup_s. + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +/* Command: MTD_LOOPIOC_TEARDOWN + * Description: Teardown a loop device previously setup vis LOOPIOC_SETUP + * Argument: A read-able pointer to the path of the device to be + * torn down + * Dependencies: The loop device must be enabled (CONFIG_MTD_DEV_LOOP=y) + */ + +#define MTD_LOOPIOC_SETUP _LOOPIOC(0x0001) +#define MTD_LOOPIOC_TEARDOWN _LOOPIOC(0x0002) + +#endif + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +/* This is the structure referred to in the argument to the LOOPIOC_SETUP + * IOCTL command. + */ + +struct mtd_losetup_s +{ + FAR const char *devname; /* The loop mtd device to be created */ + FAR const char *filename; /* The file or character device to use */ + int erasesize; /* The erase size to use on the file */ + int sectsize; /* The sector / page size of the file */ + off_t offset; /* An offset that may be applied to the device */ +}; +#endif + +/**************************************************************************** + * Public Data + ****************************************************************************/ + +#ifndef __ASSEMBLY__ + +#ifdef __cplusplus +#define EXTERN extern "C" +extern "C" +{ +#else +#define EXTERN extern +#endif + +/**************************************************************************** + * Public Function Prototypes + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +int mtd_loop_register_driver(void); Review Comment: mtd_loop_register ########## include/nuttx/fs/loopmtd.h: ########## @@ -0,0 +1,107 @@ +/**************************************************************************** + * include/nuttx/fs/loopmtd.h + * + * 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. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FS_FILEMTD_H +#define __INCLUDE_NUTTX_FS_FILEMTD_H Review Comment: __INCLUDE_NUTTX_FS_FILEMTD_H->__INCLUDE_NUTTX_FS_LOOPMTD_H ########## drivers/mtd/filemtd.c: ########## @@ -708,3 +735,201 @@ bool filemtd_isfilemtd(FAR struct mtd_dev_s *dev) return (priv->mtd.erase == filemtd_erase); } + +/**************************************************************************** + * Name: mtd_loop_register_driver + * + * Description: + * Registers MTD Loop Driver + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +int mtd_loop_register_driver(void) +{ + return register_driver("/dev/loopmtd", &g_fops, 0666, NULL); +} +#endif + +/**************************************************************************** + * Name: mtd_loop_setup + * + * Description: Dynamically setups up a FILEMTD enabled loop device that + * is backed by a file. The resulting loop device is a + * MTD type block device vs. a generic block device. + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_setup(FAR const char *devname, FAR const char *filename, + int sectsize, int erasesize, off_t offset) Review Comment: align with the previous ( ########## drivers/mtd/filemtd.c: ########## @@ -708,3 +735,201 @@ bool filemtd_isfilemtd(FAR struct mtd_dev_s *dev) return (priv->mtd.erase == filemtd_erase); } + +/**************************************************************************** + * Name: mtd_loop_register_driver + * + * Description: + * Registers MTD Loop Driver + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +int mtd_loop_register_driver(void) +{ + return register_driver("/dev/loopmtd", &g_fops, 0666, NULL); +} +#endif + +/**************************************************************************** + * Name: mtd_loop_setup + * + * Description: Dynamically setups up a FILEMTD enabled loop device that + * is backed by a file. The resulting loop device is a + * MTD type block device vs. a generic block device. + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_setup(FAR const char *devname, FAR const char *filename, + int sectsize, int erasesize, off_t offset) +{ + FAR struct mtd_dev_s *mtd; + int ret; + + mtd = filemtd_initialize(filename, offset, sectsize, erasesize); + if (mtd == NULL) + { + return -ENOENT; + } + + ret = register_mtddriver(devname, mtd, 0755, NULL); + + if (ret != OK) + { + filemtd_teardown(mtd); + } + + return ret; +} +#endif /* CONFIG_MTD_DEV_LOOP */ + +/**************************************************************************** + * Name: mtd_loop_teardown + * + * Description: + * Undo the setup performed by loopmtd_setup + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_teardown(FAR const char *devname) +{ + FAR struct file_dev_s *dev; + FAR struct inode *inode; + int ret; + + /* Sanity check */ + +#ifdef CONFIG_DEBUG_FEATURES + if (!devname) + { + return -EINVAL; + } +#endif + + /* Open the block driver associated with devname so that we can get the + * inode reference. + */ + + ret = open_blockdriver(devname, MS_RDONLY, &inode); + if (ret < 0) + { + ferr("ERROR: Failed to open %s: %d\n", devname, -ret); + return ret; + } + + /* Inode private data is a reference to the loop device structure */ + + dev = (FAR struct file_dev_s *)inode->u.i_mtd; + + /* Validate this is a filemtd backended device */ + + if (!filemtd_isfilemtd(&dev->mtd)) + { + ferr("ERROR: Device is not a FILEMTD loop: %s\n", devname); + return -EINVAL; + } + + close_blockdriver(inode); + + /* Now teardown the filemtd */ + + filemtd_teardown(&dev->mtd); + unregister_blockdriver(devname); + + kmm_free(dev); + + return OK; +} +#endif /* CONFIG_MTD_DEV_LOOP */ + +/**************************************************************************** + * Name: mtd_loop_read + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static ssize_t mtd_loop_read(FAR struct file *filep, FAR char *buffer, + size_t len) +{ + return 0; /* Return EOF */ +} +#endif /* CONFIG_MTD_DEV_LOOP */ + +/**************************************************************************** + * Name: mtd_loop_write + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static ssize_t mtd_loop_write(FAR struct file *filep, + FAR const char *buffer, size_t len) +{ + return len; /* Say that everything was written */ +} +#endif /* CONFIG_MTD_DEV_LOOP */ + +/**************************************************************************** + * Name: mtd_loop_ioctl + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_ioctl(FAR struct file *filep, int cmd, + unsigned long arg) Review Comment: remove two extra spaces ########## drivers/mtd/filemtd.c: ########## @@ -708,3 +735,201 @@ bool filemtd_isfilemtd(FAR struct mtd_dev_s *dev) return (priv->mtd.erase == filemtd_erase); } + +/**************************************************************************** + * Name: mtd_loop_register_driver + * + * Description: + * Registers MTD Loop Driver + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +int mtd_loop_register_driver(void) +{ + return register_driver("/dev/loopmtd", &g_fops, 0666, NULL); +} +#endif + +/**************************************************************************** + * Name: mtd_loop_setup + * + * Description: Dynamically setups up a FILEMTD enabled loop device that + * is backed by a file. The resulting loop device is a + * MTD type block device vs. a generic block device. + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_setup(FAR const char *devname, FAR const char *filename, + int sectsize, int erasesize, off_t offset) +{ + FAR struct mtd_dev_s *mtd; + int ret; + + mtd = filemtd_initialize(filename, offset, sectsize, erasesize); + if (mtd == NULL) + { + return -ENOENT; + } + + ret = register_mtddriver(devname, mtd, 0755, NULL); + + if (ret != OK) + { + filemtd_teardown(mtd); + } + + return ret; +} +#endif /* CONFIG_MTD_DEV_LOOP */ + +/**************************************************************************** + * Name: mtd_loop_teardown + * + * Description: + * Undo the setup performed by loopmtd_setup + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_teardown(FAR const char *devname) +{ + FAR struct file_dev_s *dev; + FAR struct inode *inode; + int ret; + + /* Sanity check */ + +#ifdef CONFIG_DEBUG_FEATURES + if (!devname) Review Comment: remove the check, done at line 918 ########## drivers/mtd/filemtd.c: ########## @@ -708,3 +735,201 @@ bool filemtd_isfilemtd(FAR struct mtd_dev_s *dev) return (priv->mtd.erase == filemtd_erase); } + +/**************************************************************************** + * Name: mtd_loop_register_driver + * + * Description: + * Registers MTD Loop Driver + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +int mtd_loop_register_driver(void) +{ + return register_driver("/dev/loopmtd", &g_fops, 0666, NULL); +} +#endif + +/**************************************************************************** + * Name: mtd_loop_setup + * + * Description: Dynamically setups up a FILEMTD enabled loop device that + * is backed by a file. The resulting loop device is a + * MTD type block device vs. a generic block device. + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_setup(FAR const char *devname, FAR const char *filename, + int sectsize, int erasesize, off_t offset) +{ + FAR struct mtd_dev_s *mtd; + int ret; + + mtd = filemtd_initialize(filename, offset, sectsize, erasesize); + if (mtd == NULL) + { + return -ENOENT; + } + + ret = register_mtddriver(devname, mtd, 0755, NULL); + + if (ret != OK) + { + filemtd_teardown(mtd); + } + + return ret; +} +#endif /* CONFIG_MTD_DEV_LOOP */ + +/**************************************************************************** + * Name: mtd_loop_teardown + * + * Description: + * Undo the setup performed by loopmtd_setup + * + ****************************************************************************/ + +#ifdef CONFIG_MTD_DEV_LOOP +static int mtd_loop_teardown(FAR const char *devname) +{ + FAR struct file_dev_s *dev; + FAR struct inode *inode; + int ret; + + /* Sanity check */ + +#ifdef CONFIG_DEBUG_FEATURES + if (!devname) + { + return -EINVAL; + } +#endif + + /* Open the block driver associated with devname so that we can get the + * inode reference. + */ + + ret = open_blockdriver(devname, MS_RDONLY, &inode); + if (ret < 0) + { + ferr("ERROR: Failed to open %s: %d\n", devname, -ret); + return ret; + } + + /* Inode private data is a reference to the loop device structure */ + + dev = (FAR struct file_dev_s *)inode->u.i_mtd; + + /* Validate this is a filemtd backended device */ + + if (!filemtd_isfilemtd(&dev->mtd)) + { + ferr("ERROR: Device is not a FILEMTD loop: %s\n", devname); + return -EINVAL; + } + + close_blockdriver(inode); + + /* Now teardown the filemtd */ + + filemtd_teardown(&dev->mtd); + unregister_blockdriver(devname); + Review Comment: remove the blank line? ########## drivers/mtd/Kconfig: ########## @@ -331,6 +331,10 @@ config FILEMTD_ERASESTATE hex "Simulated erase state" default 0xff +config MTD_DEV_LOOP Review Comment: merge to the first patch -- 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