On Wed, Apr 14, 2021 at 05:42:44PM -0300, Aline Santana Cordeiro wrote: > @@ -90,18 +92,14 @@ struct camera_mipi_info *atomisp_to_sensor_mipi_info(struct v4l2_subdev *sd) > return (struct camera_mipi_info *)v4l2_get_subdev_hostdata(sd); > } > > -/* > - * get struct atomisp_video_pipe from v4l2 video_device > - */ > +/* get struct atomisp_video_pipe from v4l2 video_device */ This code is obvious and the comment doesn't add anything except noise. Just delete it. Same for a lot of the other one line comments describing functions in this patch. > struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev) > { > return (struct atomisp_video_pipe *) > container_of(dev, struct atomisp_video_pipe, vdev); > } > > -/* > - * get struct atomisp_acc_pipe from v4l2 video_device > - */ > +/* get struct atomisp_acc_pipe from v4l2 video_device */ > struct atomisp_acc_pipe *atomisp_to_acc_pipe(struct video_device *dev) > { > return (struct atomisp_acc_pipe *) > @@ -269,7 +267,7 @@ int atomisp_freq_scaling(struct atomisp_device *isp, > ATOMISP_RUN_MODE_CONTINUOUS_CAPTURE; > } > > - /* search for the target frequency by looping freq rules*/ > + /* search for the target frequency by looping freq rules */ > for (i = 0; i < dfs->dfs_table_size; i++) { > if (curr_rules.width != dfs->dfs_table[i].width && > dfs->dfs_table[i].width != ISP_FREQ_RULE_ANY) > @@ -307,9 +305,7 @@ int atomisp_freq_scaling(struct atomisp_device *isp, > return ret; > } > > -/* > - * reset and restore ISP > - */ > +/* reset and restore ISP */ Obvious > int atomisp_reset(struct atomisp_device *isp) > { > /* Reset ISP by power-cycling it */ > @@ -338,9 +334,7 @@ int atomisp_reset(struct atomisp_device *isp) > return ret; > } > > -/* > - * interrupt disable functions > - */ > +/* interrupt disable functions */ Obvious > static void disable_isp_irq(enum hrt_isp_css_irq irq) > { > irq_disable_channel(IRQ0_ID, irq); > @@ -351,9 +345,7 @@ static void disable_isp_irq(enum hrt_isp_css_irq irq) > cnd_sp_irq_enable(SP0_ID, false); > } > > -/* > - * interrupt clean function > - */ > +/* interrupt clean function */ Obvious > static void clear_isp_irq(enum hrt_isp_css_irq irq) > { > irq_clear_all(IRQ0_ID); [ snip ] > @@ -1918,10 +1914,7 @@ irqreturn_t atomisp_isr_thread(int irq, void *isp_ptr) > return IRQ_HANDLED; > } > > -/* > - * utils for buffer allocation/free > - */ > - > +/* utils for buffer allocation/free */ What? This one seems actively wrong. > int atomisp_get_frame_pgnr(struct atomisp_device *isp, > const struct ia_css_frame *frame, u32 *p_pgnr) > { etc. regards, dan carpenter