public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@infradead.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Kevin Fitch <kfitch42@gmail.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Markus Elfring <elfring@users.sourceforge.net>,
	Javier Martinez Canillas <javier@osg.samsung.com>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>
Subject: Re: [PATCH 53/57] [media] i2c: don't break long lines
Date: Mon, 17 Oct 2016 16:54:35 +0300	[thread overview]
Message-ID: <1858182.0RpZvMcEao@avalon> (raw)
In-Reply-To: <96dbf003bc04978c5081980ab69f8b19192e3a32.1476475771.git.mchehab@s-opensource.com>

Hi Mauro,

Thank you for the patch.

On Friday 14 Oct 2016 17:20:41 Mauro Carvalho Chehab wrote:
> Due to the 80-cols checkpatch warnings, several strings
> were broken into multiple lines. This is not considered
> a good practice anymore, as it makes harder to grep for
> strings at the source code. So, join those continuation
> lines.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
>  drivers/media/i2c/as3645a.c          | 9 +++------
>  drivers/media/i2c/msp3400-kthreads.c | 3 +--
>  drivers/media/i2c/mt9m032.c          | 3 +--
>  drivers/media/i2c/mt9p031.c          | 3 +--
>  drivers/media/i2c/saa7115.c          | 3 +--
>  drivers/media/i2c/saa717x.c          | 3 +--
>  drivers/media/i2c/ths8200.c          | 4 +---
>  drivers/media/i2c/tvp5150.c          | 3 +--
>  drivers/media/i2c/tvp7002.c          | 3 +--
>  drivers/media/i2c/upd64083.c         | 3 +--
>  10 files changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/media/i2c/as3645a.c b/drivers/media/i2c/as3645a.c
> index 2e90e4094b79..95fcb8f68a1a 100644
> --- a/drivers/media/i2c/as3645a.c
> +++ b/drivers/media/i2c/as3645a.c
> @@ -299,8 +299,7 @@ static int as3645a_read_fault(struct as3645a *flash)
>  		dev_dbg(&client->dev, "Inductor Peak limit fault\n");
> 
>  	if (rval & AS_FAULT_INFO_INDICATOR_LED)
> -		dev_dbg(&client->dev, "Indicator LED fault: "
> -			"Short circuit or open loop\n");
> +		dev_dbg(&client->dev, "Indicator LED fault: Short circuit or 
open
> loop\n");
> 
>  	dev_dbg(&client->dev, "%u connected LEDs\n",
>  		rval & AS_FAULT_INFO_LED_AMOUNT ? 2 : 1);
> @@ -315,8 +314,7 @@ static int as3645a_read_fault(struct as3645a *flash)
>  		dev_dbg(&client->dev, "Short circuit fault\n");
> 
>  	if (rval & AS_FAULT_INFO_OVER_VOLTAGE)
> -		dev_dbg(&client->dev, "Over voltage fault: "
> -			"Indicates missing capacitor or open connection\n");
> +		dev_dbg(&client->dev, "Over voltage fault: Indicates missing 
capacitor or
> open connection\n");
> 
>  	return rval;
>  }
> @@ -588,8 +586,7 @@ static int as3645a_registered(struct v4l2_subdev *sd)
> 
>  	/* Verify the chip model and version. */
>  	if (model != 0x01 || rfu != 0x00) {
> -		dev_err(&client->dev, "AS3645A not detected "
> -			"(model %d rfu %d)\n", model, rfu);
> +		dev_err(&client->dev, "AS3645A not detected (model %d rfu 
%d)\n", model,
> rfu); rval = -ENODEV;
>  		goto power_off;
>  	}
> diff --git a/drivers/media/i2c/msp3400-kthreads.c
> b/drivers/media/i2c/msp3400-kthreads.c index 17120804fab7..022bea68cbf0
> 100644
> --- a/drivers/media/i2c/msp3400-kthreads.c
> +++ b/drivers/media/i2c/msp3400-kthreads.c
> @@ -775,8 +775,7 @@ int msp3410d_thread(void *data)
>  		if (msp_amsound && !state->radio &&
>  		    (state->v4l2_std & V4L2_STD_SECAM) && (val != 0x0009)) {
>  			/* autodetection has failed, let backup */
> -			v4l_dbg(1, msp_debug, client, "autodetection failed,"
> -				" switching to backup standard: %s 
(0x%04x)\n",
> +			v4l_dbg(1, msp_debug, client, "autodetection failed, 
switching to backup
> standard: %s (0x%04x)\n", msp_stdlist[8].name ?
>  					msp_stdlist[8].name : "unknown", val);
>  			state->std = val = 0x0009;
> diff --git a/drivers/media/i2c/mt9m032.c b/drivers/media/i2c/mt9m032.c
> index da076796999e..a045425887d5 100644
> --- a/drivers/media/i2c/mt9m032.c
> +++ b/drivers/media/i2c/mt9m032.c
> @@ -746,8 +746,7 @@ static int mt9m032_probe(struct i2c_client *client,
> 
>  	chip_version = mt9m032_read(client, MT9M032_CHIP_VERSION);
>  	if (chip_version != MT9M032_CHIP_VERSION_VALUE) {
> -		dev_err(&client->dev, "MT9M032 not detected, wrong version "
> -			"0x%04x\n", chip_version);
> +		dev_err(&client->dev, "MT9M032 not detected, wrong version 
0x%04x\n",
> chip_version); ret = -ENODEV;
>  		goto error_sensor;
>  	}
> diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
> index 237737fec09c..4d7b56b96a92 100644
> --- a/drivers/media/i2c/mt9p031.c
> +++ b/drivers/media/i2c/mt9p031.c
> @@ -929,8 +929,7 @@ static int mt9p031_registered(struct v4l2_subdev
> *subdev) mt9p031_power_off(mt9p031);
> 
>  	if (data != MT9P031_CHIP_VERSION_VALUE) {
> -		dev_err(&client->dev, "MT9P031 not detected, wrong version "
> -			"0x%04x\n", data);
> +		dev_err(&client->dev, "MT9P031 not detected, wrong version 
0x%04x\n",
> data); return -ENODEV;
>  	}
> 
> diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
> index 58062b41c923..5db914b7d5ae 100644
> --- a/drivers/media/i2c/saa7115.c
> +++ b/drivers/media/i2c/saa7115.c
> @@ -53,8 +53,7 @@
>  #define VRES_60HZ	(480+16)
> 
>  MODULE_DESCRIPTION("Philips SAA7111/SAA7113/SAA7114/SAA7115/SAA7118 video
> decoder driver"); -MODULE_AUTHOR(  "Maxim Yevtyushkin, Kevin Thayer, Chris
> Kennedy, " -		"Hans Verkuil, Mauro Carvalho Chehab");
> +MODULE_AUTHOR(  "Maxim Yevtyushkin, Kevin Thayer, Chris Kennedy, Hans
> Verkuil, Mauro Carvalho Chehab"); MODULE_LICENSE("GPL");
> 
>  static bool debug;
> diff --git a/drivers/media/i2c/saa717x.c b/drivers/media/i2c/saa717x.c
> index 1baca37f3eb6..b9eb152cf4e9 100644
> --- a/drivers/media/i2c/saa717x.c
> +++ b/drivers/media/i2c/saa717x.c
> @@ -735,8 +735,7 @@ static void get_inf_dev_status(struct v4l2_subdev *sd,
>  		reg_data3, stdres[reg_data3 & 0x1f],
>  		(reg_data3 & 0x000020) ? ",stereo" : "",
>  		(reg_data3 & 0x000040) ? ",dual"   : "");
> -	v4l2_dbg(1, debug, sd, "detailed status: "
> -		"%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s\n",
> +	v4l2_dbg(1, debug, sd, "detailed status:
> %s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s#%s\n", (reg_data3 & 0x000080) ? "
> A2/EIAJ pilot tone "     : "",
>  		(reg_data3 & 0x000100) ? " A2/EIAJ dual "           : "",
>  		(reg_data3 & 0x000200) ? " A2/EIAJ stereo "         : "",
> diff --git a/drivers/media/i2c/ths8200.c b/drivers/media/i2c/ths8200.c
> index 42340e364cea..69b6c32f4690 100644
> --- a/drivers/media/i2c/ths8200.c
> +++ b/drivers/media/i2c/ths8200.c
> @@ -351,9 +351,7 @@ static void ths8200_setup(struct v4l2_subdev *sd, struct
> v4l2_bt_timings *bt) /* leave reset */
>  	ths8200_s_stream(sd, true);
> 
> -	v4l2_dbg(1, debug, sd, "%s: frame %dx%d, polarity %d\n"
> -		 "horizontal: front porch %d, back porch %d, sync %d\n"
> -		 "vertical: sync %d\n", __func__, htotal(bt), vtotal(bt),
> +	v4l2_dbg(1, debug, sd, "%s: frame %dx%d, polarity %d\nhorizontal: 
front
> porch %d, back porch %d, sync %d\nvertical: sync %d\n", __func__,
> htotal(bt), vtotal(bt), polarity, bt->hfrontporch, bt->hbackporch,
>  		 bt->hsync, bt->vsync);
>  }
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 4740da39d698..827d276c8f47 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -280,8 +280,7 @@ static inline void tvp5150_selmux(struct v4l2_subdev
> *sd) break;
>  	}
> 
> -	v4l2_dbg(1, debug, sd, "Selecting video route: route input=%i, 
output=%i "
> -			"=> tvp5150 input=%i, opmode=%i\n",
> +	v4l2_dbg(1, debug, sd, "Selecting video route: route input=%i, 
output=%i
> => tvp5150 input=%i, opmode=%i\n", decoder->input, decoder->output,
>  			input, opmode);
> 
> diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
> index 3dc3341c4896..f21bda76b5ab 100644
> --- a/drivers/media/i2c/tvp7002.c
> +++ b/drivers/media/i2c/tvp7002.c
> @@ -1057,8 +1057,7 @@ static int tvp7002_remove(struct i2c_client *c)
>  	struct v4l2_subdev *sd = i2c_get_clientdata(c);
>  	struct tvp7002 *device = to_tvp7002(sd);
> 
> -	v4l2_dbg(1, debug, sd, "Removing tvp7002 adapter"
> -				"on address 0x%x\n", c->addr);
> +	v4l2_dbg(1, debug, sd, "Removing tvp7002 adapteron address 0x%x\n",
> c->addr);

You could fix errors in commit messages while at it, there's clearly a space 
missing between "adapter" and "on".

> 	v4l2_async_unregister_subdev(&device->sd);
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	media_entity_cleanup(&device->sd.entity);
> diff --git a/drivers/media/i2c/upd64083.c b/drivers/media/i2c/upd64083.c
> index 77f122f2e3c9..9b080ec4e489 100644
> --- a/drivers/media/i2c/upd64083.c
> +++ b/drivers/media/i2c/upd64083.c
> @@ -139,8 +139,7 @@ static int upd64083_log_status(struct v4l2_subdev *sd)
>  	u8 buf[7];
> 
>  	i2c_master_recv(client, buf, 7);
> -	v4l2_info(sd, "Status: SA00=%02x SA01=%02x SA02=%02x SA03=%02x "
> -		      "SA04=%02x SA05=%02x SA06=%02x\n",
> +	v4l2_info(sd, "Status: SA00=%02x SA01=%02x SA02=%02x SA03=%02x 
SA04=%02x
> SA05=%02x SA06=%02x\n", buf[0], buf[1], buf[2], buf[3], buf[4], buf[5],
> buf[6]);

I have some doubts regarding whether it makes sense to apply the rule 
explained in the commit message blindly. In this specific case for instance I 
very much doubt anyone will grep the source code for "SA03=%02x SA04=%02x". 
I'm sure this isn't the only case in this patch series where splitting 
messages in multiple lines isn't a problem.

>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart


  reply	other threads:[~2016-10-17 13:54 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-14 20:19 [PATCH 00/57] don't break long lines on strings Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 01/57] [media] b2c2: don't break long lines Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 02/57] [media] dvb-frontends: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 03/57] [media] firewire: " Mauro Carvalho Chehab
2016-10-15 11:10   ` Takashi Sakamoto
2016-10-15 13:47     ` Stefan Richter
2016-10-14 20:19 ` [PATCH 04/57] [media] cx25840: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 05/57] [media] smiapp: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 06/57] [media] soc_camera: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 07/57] [media] b2c2: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 08/57] [media] bt8xx: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 09/57] [media] cx18: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 10/57] [media] cx23885: " Mauro Carvalho Chehab
2016-10-14 20:19 ` [PATCH 11/57] [media] cx88: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 12/57] [media] ddbridge: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 13/57] [media] dm1105: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 14/57] [media] ivtv: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 15/57] [media] meye: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 16/57] [media] pt1: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 17/57] [media] saa7134: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 18/57] [media] saa7164: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 19/57] [media] solo6x10: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 20/57] [media] ttpci: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 21/57] [media] tw68: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 22/57] [media] davinci: " Mauro Carvalho Chehab
2016-10-18 16:32   ` Lad, Prabhakar
2016-10-14 20:20 ` [PATCH 23/57] [media] exynos4-is: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 24/57] [media] marvell-ccic: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 25/57] [media] omap: " Mauro Carvalho Chehab
2016-10-17 13:47   ` Laurent Pinchart
2016-10-14 20:20 ` [PATCH 26/57] [media] omap3isp: " Mauro Carvalho Chehab
2016-10-17 13:48   ` Laurent Pinchart
2016-10-14 20:20 ` [PATCH 27/57] [media] s5p-mfc: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 28/57] [media] c8sectpfe: " Mauro Carvalho Chehab
2016-10-17  6:41   ` [STLinux Kernel] " Peter Griffin
2016-10-14 20:20 ` [PATCH 29/57] [media] ti-vpe: " Mauro Carvalho Chehab
2016-10-14 21:18   ` Benoit Parrot
2016-10-14 20:20 ` [PATCH 30/57] [media] si470x: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 31/57] [media] si4713: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 32/57] [media] wl128x: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 33/57] [media] au0828: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 34/57] [media] b2c2: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 35/57] [media] cpia2: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 36/57] [media] cx231xx: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 37/57] [media] dvb-usb: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 38/57] [media] dvb-usb-v2: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 39/57] [media] em28xx: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 40/57] [media] hdpvr: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 41/57] [media] pvrusb2: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 42/57] [media] pwc: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 43/57] [media] siano: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 44/57] [media] stkwebcam: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 45/57] [media] tm6000: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 46/57] [media] ttusb-budget: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 47/57] [media] ttusb-dec: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 48/57] [media] usbvision: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 49/57] [media] uvc: " Mauro Carvalho Chehab
2016-10-17 13:49   ` Laurent Pinchart
2016-10-14 20:20 ` [PATCH 50/57] [media] zr364xx: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 51/57] [media] v4l2-core: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 52/57] [media] common: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 53/57] [media] i2c: " Mauro Carvalho Chehab
2016-10-17 13:54   ` Laurent Pinchart [this message]
2016-10-14 20:20 ` [PATCH 54/57] [media] platform: " Mauro Carvalho Chehab
2016-10-14 21:02   ` Robert Jarzmik
2016-10-17 13:45   ` Laurent Pinchart
2016-10-17 19:39     ` Andrey Utkin
2016-10-17 18:44       ` Laurent Pinchart
2016-10-17 19:50         ` Andrey Utkin
2016-10-14 20:20 ` [PATCH 55/57] [media] radio: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 56/57] [media] rc: " Mauro Carvalho Chehab
2016-10-14 20:20 ` [PATCH 57/57] [media] tuners: " Mauro Carvalho Chehab
2016-10-15 13:46 ` [PATCH 00/57] don't break long lines on strings Stefan Richter
2016-10-15 16:38   ` Mauro Carvalho Chehab

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=1858182.0RpZvMcEao@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=arnd@arndb.de \
    --cc=elfring@users.sourceforge.net \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=hans.verkuil@cisco.com \
    --cc=javier@osg.samsung.com \
    --cc=kfitch42@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@infradead.org \
    --cc=mchehab@kernel.org \
    --cc=mchehab@s-opensource.com \
    --cc=prabhakar.csengg@gmail.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox