All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Deepak R Varma <drv@mailo.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 4/9] staging: media: atomisp: use __func__ over function names
Date: Mon, 17 May 2021 15:44:48 +0200	[thread overview]
Message-ID: <20210517154448.7bb8be17@coco.lan> (raw)
In-Reply-To: <ff72157dce3b0b3f317ffb399362af7ee23109a3.1619630709.git.drv@mailo.com>

Em Wed, 28 Apr 2021 23:37:54 +0530
Deepak R Varma <drv@mailo.com> escreveu:

> Replace hard coded function names from the debug print strings by
> standard __func__ predefined identifier. This resolves following
> checkpatch script WARNING:
> Prefer using '"%s...", __func__' to using function's name, in a string.
> 
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> 
> Changes since v3:
>    - None.
> Changes since v2:
>    - None.
> Changes since v1:
>    - None.

Huh? Why are you sending a new version when there's no difference
from the past ones?

> 
>  .../staging/media/atomisp/i2c/atomisp-gc0310.c   |  2 +-
>  .../staging/media/atomisp/i2c/atomisp-gc2235.c   |  2 +-
>  .../staging/media/atomisp/i2c/atomisp-lm3554.c   |  2 +-
>  .../staging/media/atomisp/i2c/atomisp-ov2680.c   | 16 ++++++++--------
>  .../staging/media/atomisp/i2c/atomisp-ov2722.c   |  2 +-
>  5 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> index d68a2bcc9ae1..b572551f1a0d 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
> @@ -1292,7 +1292,7 @@ static int gc0310_remove(struct i2c_client *client)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
>  	struct gc0310_device *dev = to_gc0310_sensor(sd);
>  
> -	dev_dbg(&client->dev, "gc0310_remove...\n");
> +	dev_dbg(&client->dev, "%s...\n", __func__);
>  

As Dan already pointed, please delete this and other
dev_dbg() that are just tracing functions without any other
real meaning. If one needs that, ftrace could be used.



>  	dev->platform_data->csi_cfg(sd, 0);
>  
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> index e722c639b60d..548c572d3b57 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> @@ -1034,7 +1034,7 @@ static int gc2235_remove(struct i2c_client *client)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
>  	struct gc2235_device *dev = to_gc2235_sensor(sd);
>  
> -	dev_dbg(&client->dev, "gc2235_remove...\n");
> +	dev_dbg(&client->dev, "%s...\n", __func__);
>  
>  	dev->platform_data->csi_cfg(sd, 0);
>  
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
> index 7ca7378b1859..ab10fd98dbc0 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
> @@ -680,7 +680,7 @@ static int lm3554_detect(struct v4l2_subdev *sd)
>  	int ret;
>  
>  	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> -		dev_err(&client->dev, "lm3554_detect i2c error\n");
> +		dev_err(&client->dev, "%s i2c error\n", __func__);
>  		return -ENODEV;
>  	}
>  
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index f167781e258a..a51ad9843d39 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -146,7 +146,7 @@ static int ov2680_g_bin_factor_x(struct v4l2_subdev *sd, s32 *val)
>  	struct ov2680_device *dev = to_ov2680_sensor(sd);
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
>  
> -	dev_dbg(&client->dev,  "++++ov2680_g_bin_factor_x\n");
> +	dev_dbg(&client->dev,  "++++%s\n", __func__);
>  	*val = ov2680_res[dev->fmt_idx].bin_factor_x;
>  
>  	return 0;
> @@ -158,7 +158,7 @@ static int ov2680_g_bin_factor_y(struct v4l2_subdev *sd, s32 *val)
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
>  
>  	*val = ov2680_res[dev->fmt_idx].bin_factor_y;
> -	dev_dbg(&client->dev,  "++++ov2680_g_bin_factor_y\n");
> +	dev_dbg(&client->dev,  "++++%s\n", __func__);
>  	return 0;
>  }
>  
> @@ -173,7 +173,7 @@ static int ov2680_get_intg_factor(struct i2c_client *client,
>  	u16 reg_val;
>  	int ret;
>  
> -	dev_dbg(&client->dev,  "++++ov2680_get_intg_factor\n");
> +	dev_dbg(&client->dev,  "++++%s\n", __func__);
>  	if (!info)
>  		return -EINVAL;
>  
> @@ -251,8 +251,8 @@ static long __ov2680_set_exposure(struct v4l2_subdev *sd, int coarse_itg,
>  	int ret, exp_val;
>  
>  	dev_dbg(&client->dev,
> -		"+++++++__ov2680_set_exposure coarse_itg %d, gain %d, digitgain %d++\n",
> -		coarse_itg, gain, digitgain);
> +		"+++++++%s coarse_itg %d, gain %d, digitgain %d++\n",
> +		__func__, coarse_itg, gain, digitgain);

This one for instance, is not a plain trace, so it could make
sense to be kept, but please remove those "+++..."  sequences
from the string, as this has no meaning. So, just:

 	dev_dbg(&client->dev,
		"%s: coarse_itg %d, gain %d, digitgain %d\n",
		__func__, coarse_itg, gain, digitgain);

would be enough.

>  
>  	vts = ov2680_res[dev->fmt_idx].lines_per_frame;
>  
> @@ -1060,9 +1060,9 @@ static int ov2680_s_stream(struct v4l2_subdev *sd, int enable)
>  
>  	mutex_lock(&dev->input_lock);
>  	if (enable)
> -		dev_dbg(&client->dev, "ov2680_s_stream one\n");
> +		dev_dbg(&client->dev, "%s one\n", __func__);

There's a typo here:

	one -> on

Please fix.

>  	else
> -		dev_dbg(&client->dev, "ov2680_s_stream off\n");
> +		dev_dbg(&client->dev, "%s off\n", __func__);

Btw, the entire logic above could be re-written as:

	dev_dbg(&client->dev, "%s: %s\n", __func__,
		enable ? "on" : "off");

>  
>  	ret = ov2680_write_reg(client, 1, OV2680_SW_STREAM,
>  			       enable ? OV2680_START_STREAMING :
> @@ -1226,7 +1226,7 @@ static int ov2680_remove(struct i2c_client *client)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
>  	struct ov2680_device *dev = to_ov2680_sensor(sd);
>  
> -	dev_dbg(&client->dev, "ov2680_remove...\n");
> +	dev_dbg(&client->dev, "%s...\n", __func__);
>  
>  	dev->platform_data->csi_cfg(sd, 0);
>  
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> index d046a9804f63..69409f8447b5 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
> @@ -1175,7 +1175,7 @@ static int ov2722_remove(struct i2c_client *client)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(client);
>  	struct ov2722_device *dev = to_ov2722_sensor(sd);
>  
> -	dev_dbg(&client->dev, "ov2722_remove...\n");
> +	dev_dbg(&client->dev, "%s...\n", __func__);
>  
>  	dev->platform_data->csi_cfg(sd, 0);
>  	v4l2_ctrl_handler_free(&dev->ctrl_handler);



Thanks,
Mauro

  reply	other threads:[~2021-05-17 13:44 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1619630709.git.drv@mailo.com>
2021-04-28 18:06 ` [PATCH v4 3/9] staging: media: atomisp: remove unnecessary braces Deepak R Varma
2021-04-28 18:07 ` [PATCH v4 4/9] staging: media: atomisp: use __func__ over function names Deepak R Varma
2021-05-17 13:44   ` Mauro Carvalho Chehab [this message]
2021-05-19 16:08     ` Deepak R Varma
2021-04-28 18:08 ` [PATCH v4 5/9] staging: media: atomisp: reformat code comment blocks Deepak R Varma
2021-04-29  7:06   ` Fabio Aiuto
2021-04-29 11:49     ` Deepak R Varma
2021-04-30 10:04       ` Hans Verkuil
2021-04-30 11:15         ` Deepak R Varma
2021-04-30 12:27           ` Deepak R Varma
2021-04-30 12:29             ` Hans Verkuil
2021-04-28 18:09 ` [PATCH v4 6/9] staging: media: atomisp: fix CamelCase variable naming Deepak R Varma
2021-04-28 18:09 ` [PATCH v4 7/9] staging: media: atomisp: replace raw pr_*() by dev_dbg() Deepak R Varma
2021-04-28 18:10 ` [PATCH v4 8/9] staging: media: atomisp: remove unnecessary pr_info calls Deepak R Varma
2021-04-28 18:10 ` [PATCH v4 9/9] staging: media: atomisp: remove unwanted dev_*() calls 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=20210517154448.7bb8be17@coco.lan \
    --to=mchehab@kernel.org \
    --cc=drv@mailo.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=sakari.ailus@linux.intel.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.