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


##########
drivers/video/fb.c:
##########
@@ -39,6 +39,7 @@
 #include <nuttx/fs/fs.h>
 #include <nuttx/fs/ioctl.h>
 #include <nuttx/video/fb.h>
+#include <nuttx/mm/mm_map.h>

Review Comment:
   should we include it in nuttx/fs/fs.h?



##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
   return ret;
 }
 
+static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
+{
+  FAR struct inode *inode;
+  FAR struct fb_chardev_s *fb;
+  int ret = -EINVAL;
+
+  /* Get the framebuffer instance */
+
+  DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+  inode = filep->f_inode;
+  fb    = (FAR struct fb_chardev_s *)inode->i_private;
+
+  /* Return the address corresponding to the start of frame buffer. */
+
+  if (fb && map &&

Review Comment:
   let' remove fb and map check, we should trust the framework never pass the 
wrong value.



##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
   return ret;
 }
 
+static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
+{
+  FAR struct inode *inode;
+  FAR struct fb_chardev_s *fb;
+  int ret = -EINVAL;
+
+  /* Get the framebuffer instance */
+
+  DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+  inode = filep->f_inode;
+  fb    = (FAR struct fb_chardev_s *)inode->i_private;
+
+  /* Return the address corresponding to the start of frame buffer. */
+
+  if (fb && map &&
+      (size_t)map->offset + map->length < fb->fblen)

Review Comment:
   can we remove size_t cast



##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
   return ret;
 }
 
+static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
+{
+  FAR struct inode *inode;
+  FAR struct fb_chardev_s *fb;
+  int ret = -EINVAL;
+
+  /* Get the framebuffer instance */
+
+  DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+  inode = filep->f_inode;
+  fb    = (FAR struct fb_chardev_s *)inode->i_private;
+
+  /* Return the address corresponding to the start of frame buffer. */
+
+  if (fb && map &&
+      (size_t)map->offset + map->length < fb->fblen)
+    {
+      map->vaddr = (char *)fb->fbmem + map->offset;

Review Comment:
   ```suggestion
         map->vaddr = (FAR char *)fb->fbmem + map->offset;
   ```



##########
drivers/video/video.c:
##########
@@ -3200,6 +3198,21 @@ static int video_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
   return ret;
 }
 
+static int video_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR video_mng_t  *priv  = (FAR video_mng_t *)inode->i_private;
+  int ret = -EINVAL;
+
+  if (map)

Review Comment:
   if (fb && map &&
         (size_t)map->offset + map->length < fb->fblen)
       {
         map->vaddr = (char *)fb->fbmem + map->offset;
         map->vaddr = (char *)fb->fbmem + map->offset;
         map->vaddr = (FAR char *)fb->fbmem + map->offset;
         ret = OK;
       }
   
     return ret;
   



##########
drivers/video/video.c:
##########
@@ -38,6 +38,7 @@
 
 #include <nuttx/video/imgsensor.h>
 #include <nuttx/video/imgdata.h>
+#include <nuttx/mm/mm_map.h>

Review Comment:
   ditto



##########
drivers/video/fb.c:
##########
@@ -682,6 +675,30 @@ static int fb_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
   return ret;
 }
 
+static int fb_mmap(FAR struct file *filep, FAR struct mm_map_entry_s *map)
+{
+  FAR struct inode *inode;
+  FAR struct fb_chardev_s *fb;
+  int ret = -EINVAL;
+
+  /* Get the framebuffer instance */
+
+  DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+  inode = filep->f_inode;
+  fb    = (FAR struct fb_chardev_s *)inode->i_private;
+
+  /* Return the address corresponding to the start of frame buffer. */
+
+  if (fb && map &&
+      (size_t)map->offset + map->length < fb->fblen)

Review Comment:
   ```suggestion
         (size_t)map->offset + map->length <= fb->fblen)
   ```



-- 
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