Re: [PATCH v2] staging: media: atomisp: pci: Format comments according to coding-style in file atomisp_cmd.c
Em qui, 2021-04-15 às 08:48 +0300, Dan Carpenter escreveu: > 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. > No worries, I'm going to delete it all. Can I send a v3 just with the issues detected by checkpatch? Thank you in advance, Aline > > 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 >
Re: [PATCH v2] staging: media: atomisp: pci: Format comments according to coding-style in file atomisp_cmd.c
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
Re: [PATCH v2] staging: media: atomisp: pci: Format comments according to coding-style in file atomisp_cmd.c
On Wed, Apr 14, 2021 at 05:42:44PM -0300, Aline Santana Cordeiro wrote: > Format all comments according to the coding-style. > Issue detected by checkpatch.pl. > > Signed-off-by: Aline Santana Cordeiro Thanks! Acked-by: Sakari Ailus -- Sakari Ailus
[PATCH v2] staging: media: atomisp: pci: Format comments according to coding-style in file atomisp_cmd.c
Format all comments according to the coding-style. Issue detected by checkpatch.pl. Signed-off-by: Aline Santana Cordeiro --- Changes since v1: - Stantardize all the multi-line and single-line comments drivers/staging/media/atomisp/pci/atomisp_cmd.c | 377 +++- 1 file changed, 169 insertions(+), 208 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c index 592ea99..abc17ec 100644 --- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c +++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c @@ -63,11 +63,13 @@ #include "ia_css_debug.h" #include "bits.h" -/* We should never need to run the flash for more than 2 frames. +/* + * We should never need to run the flash for more than 2 frames. * At 15fps this means 133ms. We set the timeout a bit longer. * Each flash driver is supposed to set its own timeout, but * just in case someone else changed the timeout, we set it - * here to make sure we don't damage the flash hardware. */ + * here to make sure we don't damage the flash hardware. + */ #define FLASH_TIMEOUT 800 /* ms */ union host { @@ -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 */ 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 */ 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 */ 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 */ static void clear_isp_irq(enum hrt_isp_css_irq irq) { irq_clear_all(IRQ0_ID); @@ -514,7 +506,7 @@ __get_asd_from_port(struct atomisp_device *isp, enum mipi_port_id port) return NULL; } -/* interrupt handling function*/ +/* interrupt handling function */ irqreturn_t atomisp_isr(int irq, void *dev) { struct atomisp_device *isp = (struct atomisp_device *)dev; @@ -555,14 +547,16 @@ irqreturn_t atomisp_isr(int irq, void *dev) atomic_inc(>sof_count); atomisp_sof_event(asd); - /* If sequence_temp and sequence are the same + /* +* If sequence_temp and sequence are the same * there where no frames lost so we can increase * sequence_temp. * If not then processing of frame is still in progress * and driver needs to keep old sequence_temp value. * NOTE: There is assumption here that ISP will not * start processing next frame from sensor before old -* one is completely done. */ +* one is completely done. +*/ if (atomic_read(>sequence) == atomic_read( >sequence_temp)) atomic_set(>sequence_temp, @@ -812,9 +806,7 @@ static struct atomisp_video_pipe *__atomisp_get_pipe( */ return >video_out_video_capture; } else if (css_pipe_id == IA_CSS_PIPE_ID_YUVPP) { - /* -* to SOC camera, yuvpp pipe is run for capture/video/SDV/ZSL. -*/ + /* to SOC camera, yuvpp pipe is run for capture/video/SDV/ZSL. */ if (asd->continuous_mode->val) {