Hi Deepak, If you're touching all these lines, I might do a little more. Please see the comments below.
On Fri, Apr 30, 2021 at 09:10:12PM +0530, Deepak R Varma wrote: > Misplaced braces makes it difficult to follow the code easily. This also > goes against the code style guidelines. This resolved following checkpatch > complaints: > > ERROR: open brace '{' following function definitions go on the next line > ERROR: that open brace { should be on the previous line > > Signed-off-by: Deepak R Varma <d...@mailo.com> > --- > > Please Note: There are several other checkpatch triggered warnings and > errors that should be addressed in separate patches. > > > .../staging/media/atomisp/pci/atomisp_csi2.c | 3 +- > .../staging/media/atomisp/pci/sh_css_mipi.c | 69 +++---- > .../staging/media/atomisp/pci/sh_css_params.c | 171 ++++++++---------- > drivers/staging/media/atomisp/pci/sh_css_sp.c | 108 +++++------ > .../media/atomisp/pci/sh_css_version.c | 3 +- > 5 files changed, 157 insertions(+), 197 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c > b/drivers/staging/media/atomisp/pci/atomisp_csi2.c > index 060b8765ae96..200f16994f3a 100644 > --- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c > +++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c > @@ -29,7 +29,8 @@ static struct v4l2_mbus_framefmt *__csi2_get_format(struct > v4l2_subdev_pad_config *cfg, > enum > v4l2_subdev_format_whence You could have more arguments on the same line up to 80 characters per line. > - which, unsigned int pad) { > + which, unsigned int pad) > +{ > if (which == V4L2_SUBDEV_FORMAT_TRY) > return v4l2_subdev_get_try_format(&csi2->subdev, cfg, pad); > else > diff --git a/drivers/staging/media/atomisp/pci/sh_css_mipi.c > b/drivers/staging/media/atomisp/pci/sh_css_mipi.c > index 3f34cc81be87..75489f7d75ee 100644 > --- a/drivers/staging/media/atomisp/pci/sh_css_mipi.c > +++ b/drivers/staging/media/atomisp/pci/sh_css_mipi.c > @@ -91,7 +91,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width, > const enum atomisp_input_format format, > const bool hasSOLandEOL, > const unsigned int embedded_data_size_words, > - unsigned int *size_mem_words) { > + unsigned int *size_mem_words) > +{ > int err = 0; > > unsigned int bits_per_pixel = 0; > @@ -118,8 +119,7 @@ ia_css_mipi_frame_calculate_size(const unsigned int width, > IA_CSS_ENTER("padded_width=%d, height=%d, format=%d, hasSOLandEOL=%d, > embedded_data_size_words=%d\n", > width_padded, height, format, hasSOLandEOL, > embedded_data_size_words); > > - switch (format) > - { > + switch (format) { > case ATOMISP_INPUT_FORMAT_RAW_6: /* 4p, 3B, 24bits */ > bits_per_pixel = 6; > break; > @@ -178,12 +178,10 @@ ia_css_mipi_frame_calculate_size(const unsigned int > width, > /* Even lines for YUV420 formats are double in bits_per_pixel. */ > if (format == ATOMISP_INPUT_FORMAT_YUV420_8 > || format == ATOMISP_INPUT_FORMAT_YUV420_10 > - || format == ATOMISP_INPUT_FORMAT_YUV420_16) > - { > + || format == ATOMISP_INPUT_FORMAT_YUV420_16) { > even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >> > 3; /* ceil ( bits per line / 8) */ > - } else > - { > + } else { > even_line_bytes = odd_line_bytes; > } > > @@ -236,7 +234,8 @@ ia_css_mipi_frame_calculate_size(const unsigned int width, > #if !defined(ISP2401) > int > ia_css_mipi_frame_enable_check_on_size(const enum mipi_port_id port, > - const unsigned int size_mem_words) > { > + const unsigned int size_mem_words) Tab after int, should be a space. > +{ > u32 idx; > > int err = -EBUSY; > @@ -246,11 +245,9 @@ ia_css_mipi_frame_enable_check_on_size(const enum > mipi_port_id port, > > for (idx = 0; idx < IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT && > my_css.mipi_sizes_for_check[port][idx] != 0; > - idx++) /* do nothing */ > - { > + idx++) { /* do nothing */ > } A semicolon suffices here. I guess this could be also written as: for (idx = 0; idx < ... & ... != 0; ) idx++; As it's easy to just miss the semicolon at the end of the for loop. > - if (idx < IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT) > - { > + if (idx < IA_CSS_MIPI_SIZE_CHECK_MAX_NOF_ENTRIES_PER_PORT) { > my_css.mipi_sizes_for_check[port][idx] = size_mem_words; > err = 0; > } > @@ -271,7 +268,8 @@ mipi_init(void) > int > calculate_mipi_buff_size( > struct ia_css_stream_config *stream_cfg, > - unsigned int *size_mem_words) { > + unsigned int *size_mem_words) I think both arguments might fit on the same line. If not, then the return type (int) does. I think there are other similar cases. > +{ > #if !defined(ISP2401) > int err = -EINVAL; > (void)stream_cfg; > @@ -346,12 +344,10 @@ calculate_mipi_buff_size( > > /* Even lines for YUV420 formats are double in bits_per_pixel. */ > if (format == ATOMISP_INPUT_FORMAT_YUV420_8 > - || format == ATOMISP_INPUT_FORMAT_YUV420_10) > - { > + || format == ATOMISP_INPUT_FORMAT_YUV420_10) { > even_line_bytes = (width_padded * 2 * bits_per_pixel + 7) >> > 3; /* ceil ( bits per line / 8) */ You don't need braces here. > - } else > - { > + } else { > even_line_bytes = odd_line_bytes; > } > > @@ -393,7 +389,8 @@ static bool buffers_needed(struct ia_css_pipe *pipe) > > int > allocate_mipi_frames(struct ia_css_pipe *pipe, > - struct ia_css_stream_info *info) { > + struct ia_css_stream_info *info) > +{ > int err = -EINVAL; > unsigned int port; > > @@ -402,8 +399,7 @@ allocate_mipi_frames(struct ia_css_pipe *pipe, > > assert(pipe); > assert(pipe->stream); > - if ((!pipe) || (!pipe->stream)) > - { > + if ((!pipe) || (!pipe->stream)) { Extra parentheses. -- Kind regards, Sakari Ailus _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel