xiaoxiang781216 commented on code in PR #16624: URL: https://github.com/apache/nuttx/pull/16624#discussion_r2167398619
########## drivers/mtd/mtd_register.c: ########## @@ -0,0 +1,516 @@ +/**************************************************************************** + * drivers/mtd/mtd_register.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 <sys/types.h> +#include <inttypes.h> +#include <stdint.h> +#include <stdlib.h> +#include <errno.h> +#include <assert.h> +#include <debug.h> +#include <string.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdbool.h> +#include <sys/stat.h> + +#include <nuttx/mtd/mtd.h> +#include <nuttx/kmalloc.h> +#include <nuttx/fs/ioctl.h> +#ifdef CONFIG_FS_PROCFS +#include <nuttx/fs/procfs.h> Review Comment: ```suggestion # include <nuttx/fs/procfs.h> ``` ########## drivers/mtd/Make.defs: ########## @@ -41,6 +41,10 @@ ifeq ($(CONFIG_MTD_PARTITION),y) CSRCS += mtd_partition.c endif +ifeq ($(CONFIG_MTD_PARTITION_REGISTER),y) +CSRCS += mtd_register.c Review Comment: let's modify cmake too ########## drivers/mtd/mtd_register.c: ########## @@ -0,0 +1,516 @@ +/**************************************************************************** + * drivers/mtd/mtd_register.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 <sys/types.h> +#include <inttypes.h> +#include <stdint.h> +#include <stdlib.h> +#include <errno.h> +#include <assert.h> +#include <debug.h> +#include <string.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdbool.h> +#include <sys/stat.h> + +#include <nuttx/mtd/mtd.h> +#include <nuttx/kmalloc.h> +#include <nuttx/fs/ioctl.h> +#ifdef CONFIG_FS_PROCFS +#include <nuttx/fs/procfs.h> +#endif + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct mtd_struct_s +{ + FAR struct mtd_dev_s *mtd; /* MTD layer representing flash partition */ + uint32_t blksize; /* Size of one write page */ + uint16_t refs; /* Number of references */ + uint8_t erasestate; /* Erase state of flash partition */ + size_t size; /* Size of the partition in bytes */ + mutex_t lock; /* Lock for the driver access */ + bool readonly; /* True if the partition supposed to be + * read only. This will block write access. + */ +}; + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int mtd_open(FAR struct file *filep); +static int mtd_close(FAR struct file *filep); +static off_t mtd_seek(FAR struct file *filep, off_t offset, int whence); +static ssize_t mtd_read(FAR struct file *filep, FAR char *buffer, + size_t buflen); +static ssize_t mtd_write(FAR struct file *filep, FAR const char *buffer, + size_t buflen); +static int mtd_ioctl(FAR struct file *filep, int cmd, + unsigned long arg); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct file_operations g_fops = +{ + mtd_open, /* open */ + mtd_close, /* close */ + mtd_read, /* read */ + mtd_write, /* write */ + mtd_seek, /* seek */ + mtd_ioctl /* ioctl */ +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: mtd_open + * + * Description: Open the block device + * + ****************************************************************************/ + +static int mtd_open(FAR struct file *filep) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *mtd; + int ret = OK; + + DEBUGASSERT(inode->i_private); + mtd = inode->i_private; + + /* Increment the reference count */ + + ret = nxmutex_lock(&mtd->lock); + if (ret < 0) + { + return ret; + } + + if (mtd->refs == 255) + { + ret = -EMFILE; + } + else + { + mtd->refs++; + } + + nxmutex_unlock(&mtd->lock); + return ret; +} + +/**************************************************************************** + * Name: mtd_close + * + * Description: close the block device + * + ****************************************************************************/ + +static int mtd_close(FAR struct file *filep) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *mtd; + int ret = OK; + + DEBUGASSERT(inode->i_private); + mtd = inode->i_private; + + /* Get exclusive access */ + + ret = nxmutex_lock(&mtd->lock); + if (ret < 0) + { + return ret; + } + + /* Decrement the reference count */ + + if (mtd->refs == 0) + { + ret = -EIO; + } + else + { + mtd->refs--; + } + + nxmutex_unlock(&mtd->lock); + return ret; +} + +/**************************************************************************** + * Name: mtd_read + * + * Description: Read the specified number of bytes. + * + ****************************************************************************/ + +static ssize_t mtd_read(FAR struct file *filep, FAR char *buffer, size_t len) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *dev; + FAR char *buf; + off_t startblock; + off_t offset; + size_t nblocks; + ssize_t ret; + + DEBUGASSERT(inode->i_private); + dev = inode->i_private; + + if (len < 1) + { + return 0; + } + + ret = nxmutex_lock(&dev->lock); + if (ret < 0) + { + return ret; + } + + if (filep->f_pos >= dev->size) + { + /* End of file */ + + nxmutex_unlock(&dev->lock); + return 0; + } + + ret = MTD_READ(dev->mtd, filep->f_pos, len, (FAR uint8_t *)buffer); + if (ret < 0) + { + if (ret == -ENOSYS) + { + /* Byte read not supported, use block read */ + + startblock = filep->f_pos / dev->blksize; + nblocks = (len / dev->blksize) + 1; + buf = kmm_zalloc(sizeof(char) * (nblocks * dev->blksize)); Review Comment: could we preallocate the buffer to avoid alloc/free buffer for each unaligned access ########## drivers/mtd/mtd_register.c: ########## @@ -0,0 +1,516 @@ +/**************************************************************************** + * drivers/mtd/mtd_register.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 <sys/types.h> +#include <inttypes.h> +#include <stdint.h> +#include <stdlib.h> +#include <errno.h> +#include <assert.h> +#include <debug.h> +#include <string.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdbool.h> +#include <sys/stat.h> + +#include <nuttx/mtd/mtd.h> +#include <nuttx/kmalloc.h> +#include <nuttx/fs/ioctl.h> +#ifdef CONFIG_FS_PROCFS +#include <nuttx/fs/procfs.h> +#endif + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct mtd_struct_s +{ + FAR struct mtd_dev_s *mtd; /* MTD layer representing flash partition */ + uint32_t blksize; /* Size of one write page */ + uint16_t refs; /* Number of references */ + uint8_t erasestate; /* Erase state of flash partition */ + size_t size; /* Size of the partition in bytes */ + mutex_t lock; /* Lock for the driver access */ + bool readonly; /* True if the partition supposed to be + * read only. This will block write access. + */ +}; + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int mtd_open(FAR struct file *filep); +static int mtd_close(FAR struct file *filep); +static off_t mtd_seek(FAR struct file *filep, off_t offset, int whence); +static ssize_t mtd_read(FAR struct file *filep, FAR char *buffer, + size_t buflen); +static ssize_t mtd_write(FAR struct file *filep, FAR const char *buffer, + size_t buflen); +static int mtd_ioctl(FAR struct file *filep, int cmd, + unsigned long arg); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static const struct file_operations g_fops = +{ + mtd_open, /* open */ + mtd_close, /* close */ + mtd_read, /* read */ + mtd_write, /* write */ + mtd_seek, /* seek */ + mtd_ioctl /* ioctl */ +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: mtd_open + * + * Description: Open the block device + * + ****************************************************************************/ + +static int mtd_open(FAR struct file *filep) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *mtd; + int ret = OK; + + DEBUGASSERT(inode->i_private); + mtd = inode->i_private; + + /* Increment the reference count */ + + ret = nxmutex_lock(&mtd->lock); + if (ret < 0) + { + return ret; + } + + if (mtd->refs == 255) + { + ret = -EMFILE; + } + else + { + mtd->refs++; + } + + nxmutex_unlock(&mtd->lock); + return ret; +} + +/**************************************************************************** + * Name: mtd_close + * + * Description: close the block device + * + ****************************************************************************/ + +static int mtd_close(FAR struct file *filep) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *mtd; + int ret = OK; + + DEBUGASSERT(inode->i_private); + mtd = inode->i_private; + + /* Get exclusive access */ + + ret = nxmutex_lock(&mtd->lock); + if (ret < 0) + { + return ret; + } + + /* Decrement the reference count */ + + if (mtd->refs == 0) + { + ret = -EIO; + } + else + { + mtd->refs--; + } + + nxmutex_unlock(&mtd->lock); + return ret; +} + +/**************************************************************************** + * Name: mtd_read + * + * Description: Read the specified number of bytes. + * + ****************************************************************************/ + +static ssize_t mtd_read(FAR struct file *filep, FAR char *buffer, size_t len) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *dev; + FAR char *buf; + off_t startblock; + off_t offset; + size_t nblocks; + ssize_t ret; + + DEBUGASSERT(inode->i_private); + dev = inode->i_private; + + if (len < 1) + { + return 0; + } + + ret = nxmutex_lock(&dev->lock); + if (ret < 0) + { + return ret; + } + + if (filep->f_pos >= dev->size) + { + /* End of file */ + + nxmutex_unlock(&dev->lock); + return 0; + } + + ret = MTD_READ(dev->mtd, filep->f_pos, len, (FAR uint8_t *)buffer); + if (ret < 0) + { + if (ret == -ENOSYS) + { + /* Byte read not supported, use block read */ + + startblock = filep->f_pos / dev->blksize; + nblocks = (len / dev->blksize) + 1; + buf = kmm_zalloc(sizeof(char) * (nblocks * dev->blksize)); + if (buf) + { + offset = filep->f_pos - (startblock * dev->blksize); + ret = MTD_BREAD(dev->mtd, startblock, nblocks, + (FAR uint8_t *)buffer); + ret *= dev->blksize; + if (ret >= offset) + { + memcpy(buffer, buf + offset, len); + ret -= offset; + filep->f_pos += ret > len ? len : ret; + } + else + { + /* We haven't read enough bytes to obtain the desired + * offset, return EOF. + */ + + ret = 0; + } + + kmm_free(buf); + } + else + { + ret = -ENOMEM; + } + } + } + else + { + filep->f_pos += ret; + } + + nxmutex_unlock(&dev->lock); + return ret; +} + +/**************************************************************************** + * Name: mtd_write + * + * Description: Read the specified number of bytes + * + ****************************************************************************/ + +static ssize_t mtd_write(FAR struct file *filep, FAR const char *buffer, + size_t len) +{ + FAR struct inode *inode = filep->f_inode; + FAR struct mtd_struct_s *dev; + off_t offset; + off_t startblock; + size_t nblocks; + FAR char *buf; + ssize_t ret = -EACCES; + + DEBUGASSERT(inode->i_private); + dev = inode->i_private; + + if (dev->readonly) + { + return -EACCES; + } + + if (len < 1) + { + return 0; + } + + ret = nxmutex_lock(&dev->lock); + if (ret < 0) + { + return ret; + } + + if (filep->f_pos >= dev->size) + { + nxmutex_unlock(&dev->lock); + return -EFBIG; + } + +#ifdef CONFIG_MTD_BYTE_WRITE + ret = MTD_WRITE(dev->mtd, filep->f_pos, len, (FAR uint8_t *)buffer); + if (ret == -ENOSYS) +#endif + { + startblock = filep->f_pos / dev->blksize; + nblocks = (len / dev->blksize) + 1; + buf = kmm_zalloc(sizeof(char) * (nblocks * dev->blksize)); Review Comment: remove sizeof(char) here and line 230 -- 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
