From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: Deepak R Varma <drv@mailo.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-media@vger.kernel.org, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: media: atomisp: pci: reposition braces as per coding style
Date: Fri, 30 Apr 2021 19:33:27 +0300 [thread overview]
Message-ID: <20210430163327.GI3@paasikivi.fi.intel.com> (raw)
In-Reply-To: <YIwk3KbVGRPJwKa4@dU2104>
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 <drv@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
next prev parent reply other threads:[~2021-04-30 16:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-30 15:40 [PATCH] staging: media: atomisp: pci: reposition braces as per coding style Deepak R Varma
2021-04-30 16:33 ` Sakari Ailus [this message]
2021-04-30 17:04 ` Deepak R Varma
2021-04-30 17:12 ` Sakari Ailus
2021-04-30 17:17 ` Deepak R Varma
2021-05-04 15:48 ` Deepak R Varma
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210430163327.GI3@paasikivi.fi.intel.com \
--to=sakari.ailus@linux.intel.com \
--cc=devel@driverdev.osuosl.org \
--cc=drv@mailo.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).