Re: [PATCH v2 54/58] i2c: don't break long lines

[Mauro Carvalho Chehab]

Em Thu, 20 Oct 2016 13:46:08 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Tuesday 18 Oct 2016 18:46:06 Mauro Carvalho Chehab wrote:
> > Due to the 80-cols restrictions, and latter due to 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.
> > 
> > As we're right now fixing other drivers due to KERN_CONT, we need
> > to be able to identify what printk strings don't end with a "\n".
> > It is a way easier to detect those if we don't break long lines.
> > 
> > So, join those continuation lines.
> > 
> > The patch was generated via the script below, and manually
> > adjusted if needed.
> > 
> > 
> > use Text::Tabs;
> > while (<>) {
> > if ($next ne "") {
> > $c=$_;
> > if ($c =~ /^\s+\"(.*)/) {
> > $c2=$1;
> > $next =~ s/\"\n$//;
> > $n = expand($next);
> > $funpos = index($n, '(');
> > $pos = index($c2, '",');
> > if ($funpos && $pos > 0) {
> > $s1 = substr $c2, 0, $pos + 2;
> > $s2 = ' ' x ($funpos + 1) . substr $c2,   
> $pos + 2;
> > $s2 =~ s/^\s+//;
> > 
> > $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne   
> "");
> > 
> > print unexpand("$next$s1\n");
> > print unexpand("$s2\n") if ($s2 ne "");
> > } else {
> > print "$next$c2\n";
> > }
> > $next="";
> > next;
> > } else {
> > print $next;
> > }
> > $next="";
> > } else {
> > if (m/\"$/) {
> > if (!m/\\n\"$/) {
> > $next=$_;
> > next;
> > }
> > }
> > }
> > print $_;
> > }
> > 
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/i2c/as3645a.c  | 13 +++--
> >  drivers/media/i2c/msp3400-kthreads.c |  4 ++--
> >  drivers/media/i2c/mt9m032.c  |  5 +++--
> >  drivers/media/i2c/mt9p031.c  |  5 +++--
> >  drivers/media/i2c/saa7115.c  | 18 +++---
> >  drivers/media/i2c/saa717x.c  |  4 ++--
> >  drivers/media/i2c/tvp5150.c  | 14 --
> >  drivers/media/i2c/tvp7002.c  |  6 +++---
> >  drivers/media/i2c/upd64083.c |  4 +---
> >  9 files changed, 40 insertions(+), 33 deletions(-)  
> 
> [snip]
> 
> > diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
> > index 58062b41c923..3b341a9da004 100644
> > --- a/drivers/media/i2c/saa7115.c
> > +++ b/drivers/media/i2c/saa7115.c  
> 
> [snip]
> 
> 
> > @@ -1538,8 +1537,10 @@ static int saa711x_log_status(struct v4l2_subdev *sd)
> > /* status for the saa7114 */
> > reg1f = saa711x_read(sd, R_1F_STATUS_BYTE_2_VD_DEC);
> > signalOk = (reg1f & 0xc1) == 0x81;
> > -   v4l2_info(sd, "Video signal:%s\n", signalOk ? "ok" :   
> "bad");
> 
> No need to change this one, if fits on a single line.

I know, but visually, it looked better to make the same indentation
as on the frequency print below.

> 
> > -   v4l2_info(sd, "Frequency:   %s\n", (reg1f & 0x20) ? "60   
> Hz" : "50
> > Hz"); + v4l2_info(sd, "Video signal:%s\n",
> > + signalOk ? "ok" : "bad");
> > +   v4l2_info(sd, "Frequency:   %s\n",
> > + (reg1f & 0x20) ? "60 Hz" : "50 Hz");
> > return 0;
> > }
> >   
> 
> [snip]
> 
> > diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> > index 4740da39d698..b3a9580ef1e4 100644
> > --- a/drivers/media/i2c/tvp5150.c
> > +++ b/drivers/media/i2c/tvp5150.c
> > @@ -280,10 +280,10 @@ 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",  
> > -   decoder->input, decoder->output,
> > -   input, opmode);
> > +   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);  
> 
> The three arguments can fit on a single line.
> 
> > 
> > tvp5150_write(sd, TVP5150_OP_MODE_CTL, opmode);
> > tvp5150_write(sd, TVP5150_VD_IN_SRC_SEL_1, input);
> > @@ -649,7 +649,8 @@ static int tvp5150_set_vbi(struct v4l2_subdev *sd,
> > int pos=0;
> > 
> > if (std == V4L2_STD_ALL) {
> > 

Re: [PATCH v2 54/58] i2c: don't break long lines

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Tuesday 18 Oct 2016 18:46:06 Mauro Carvalho Chehab wrote:
> Due to the 80-cols restrictions, and latter due to 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.
> 
> As we're right now fixing other drivers due to KERN_CONT, we need
> to be able to identify what printk strings don't end with a "\n".
> It is a way easier to detect those if we don't break long lines.
> 
> So, join those continuation lines.
> 
> The patch was generated via the script below, and manually
> adjusted if needed.
> 
> 
> use Text::Tabs;
> while (<>) {
>   if ($next ne "") {
>   $c=$_;
>   if ($c =~ /^\s+\"(.*)/) {
>   $c2=$1;
>   $next =~ s/\"\n$//;
>   $n = expand($next);
>   $funpos = index($n, '(');
>   $pos = index($c2, '",');
>   if ($funpos && $pos > 0) {
>   $s1 = substr $c2, 0, $pos + 2;
>   $s2 = ' ' x ($funpos + 1) . substr $c2, 
$pos + 2;
>   $s2 =~ s/^\s+//;
> 
>   $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne 
"");
> 
>   print unexpand("$next$s1\n");
>   print unexpand("$s2\n") if ($s2 ne "");
>   } else {
>   print "$next$c2\n";
>   }
>   $next="";
>   next;
>   } else {
>   print $next;
>   }
>   $next="";
>   } else {
>   if (m/\"$/) {
>   if (!m/\\n\"$/) {
>   $next=$_;
>   next;
>   }
>   }
>   }
>   print $_;
> }
> 
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/i2c/as3645a.c  | 13 +++--
>  drivers/media/i2c/msp3400-kthreads.c |  4 ++--
>  drivers/media/i2c/mt9m032.c  |  5 +++--
>  drivers/media/i2c/mt9p031.c  |  5 +++--
>  drivers/media/i2c/saa7115.c  | 18 +++---
>  drivers/media/i2c/saa717x.c  |  4 ++--
>  drivers/media/i2c/tvp5150.c  | 14 --
>  drivers/media/i2c/tvp7002.c  |  6 +++---
>  drivers/media/i2c/upd64083.c |  4 +---
>  9 files changed, 40 insertions(+), 33 deletions(-)

[snip]

> diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c
> index 58062b41c923..3b341a9da004 100644
> --- a/drivers/media/i2c/saa7115.c
> +++ b/drivers/media/i2c/saa7115.c

[snip]


> @@ -1538,8 +1537,10 @@ static int saa711x_log_status(struct v4l2_subdev *sd)
> /* status for the saa7114 */
>   reg1f = saa711x_read(sd, R_1F_STATUS_BYTE_2_VD_DEC);
>   signalOk = (reg1f & 0xc1) == 0x81;
> - v4l2_info(sd, "Video signal:%s\n", signalOk ? "ok" : 
"bad");

No need to change this one, if fits on a single line.

> - v4l2_info(sd, "Frequency:   %s\n", (reg1f & 0x20) ? "60 
Hz" : "50
> Hz"); +   v4l2_info(sd, "Video signal:%s\n",
> +   signalOk ? "ok" : "bad");
> + v4l2_info(sd, "Frequency:   %s\n",
> +   (reg1f & 0x20) ? "60 Hz" : "50 Hz");
>   return 0;
>   }
> 

[snip]

> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 4740da39d698..b3a9580ef1e4 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -280,10 +280,10 @@ 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",
> - decoder->input, decoder->output,
> - input, opmode);
> + 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);

The three arguments can fit on a single line.

> 
>   tvp5150_write(sd, TVP5150_OP_MODE_CTL, opmode);
>   tvp5150_write(sd, TVP5150_VD_IN_SRC_SEL_1, input);
> @@ -649,7 +649,8 @@ static int tvp5150_set_vbi(struct v4l2_subdev *sd,
>   int pos=0;
> 
>   if (std == V4L2_STD_ALL) {
> - v4l2_err(sd, "VBI can't be configured without knowing number 
of
> lines\n");
> + v4l2_err(sd,
> +  "VBI can't be configured without knowing number of 
lines\n");

I'm quite doubtful that this particular change improves readability :-)

>   return 0;
>   } else if (std & V4L2_STD_625_50) {
>   /* Don't 

Re: [PATCH v2 54/58] i2c: don't break long lines

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.


On Tue, Oct 18, 2016 at 9:46 PM, Mauro Carvalho Chehab
 wrote:
> Due to the 80-cols restrictions, and latter due to 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.
>
> As we're right now fixing other drivers due to KERN_CONT, we need
> to be able to identify what printk strings don't end with a "\n".
> It is a way easier to detect those if we don't break long lines.
>
> So, join those continuation lines.
>
> The patch was generated via the script below, and manually
> adjusted if needed.
>
> 
> use Text::Tabs;
> while (<>) {
> if ($next ne "") {
> $c=$_;
> if ($c =~ /^\s+\"(.*)/) {
> $c2=$1;
> $next =~ s/\"\n$//;
> $n = expand($next);
> $funpos = index($n, '(');
> $pos = index($c2, '",');
> if ($funpos && $pos > 0) {
> $s1 = substr $c2, 0, $pos + 2;
> $s2 = ' ' x ($funpos + 1) . substr $c2, $pos 
> + 2;
> $s2 =~ s/^\s+//;
>
> $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne 
> "");
>
> print unexpand("$next$s1\n");
> print unexpand("$s2\n") if ($s2 ne "");
> } else {
> print "$next$c2\n";
> }
> $next="";
> next;
> } else {
> print $next;
> }
> $next="";
> } else {
> if (m/\"$/) {
> if (!m/\\n\"$/) {
> $next=$_;
> next;
> }
> }
> }
> print $_;
> }
> 
>
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Lad, Prabhakar 

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 54/58] i2c: don't break long lines

[Mauro Carvalho Chehab]

Due to the 80-cols restrictions, and latter due to 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.

As we're right now fixing other drivers due to KERN_CONT, we need
to be able to identify what printk strings don't end with a "\n".
It is a way easier to detect those if we don't break long lines.

So, join those continuation lines.

The patch was generated via the script below, and manually
adjusted if needed.


use Text::Tabs;
while (<>) {
if ($next ne "") {
$c=$_;
if ($c =~ /^\s+\"(.*)/) {
$c2=$1;
$next =~ s/\"\n$//;
$n = expand($next);
$funpos = index($n, '(');
$pos = index($c2, '",');
if ($funpos && $pos > 0) {
$s1 = substr $c2, 0, $pos + 2;
$s2 = ' ' x ($funpos + 1) . substr $c2, $pos + 
2;
$s2 =~ s/^\s+//;

$s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne "");

print unexpand("$next$s1\n");
print unexpand("$s2\n") if ($s2 ne "");
} else {
print "$next$c2\n";
}
$next="";
next;
} else {
print $next;
}
$next="";
} else {
if (m/\"$/) {
if (!m/\\n\"$/) {
$next=$_;
next;
}
}
}
print $_;
}


Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/as3645a.c  | 13 +++--
 drivers/media/i2c/msp3400-kthreads.c |  4 ++--
 drivers/media/i2c/mt9m032.c  |  5 +++--
 drivers/media/i2c/mt9p031.c  |  5 +++--
 drivers/media/i2c/saa7115.c  | 18 +++---
 drivers/media/i2c/saa717x.c  |  4 ++--
 drivers/media/i2c/tvp5150.c  | 14 --
 drivers/media/i2c/tvp7002.c  |  6 +++---
 drivers/media/i2c/upd64083.c |  4 +---
 9 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/drivers/media/i2c/as3645a.c b/drivers/media/i2c/as3645a.c
index 2e90e4094b79..d2f56014a150 100644
--- a/drivers/media/i2c/as3645a.c
+++ b/drivers/media/i2c/as3645a.c
@@ -299,8 +299,8 @@ static int as3645a_read_fault(struct as3645a *flash)
dev_dbg(>dev, "Inductor Peak limit fault\n");
 
if (rval & AS_FAULT_INFO_INDICATOR_LED)
-   dev_dbg(>dev, "Indicator LED fault: "
-   "Short circuit or open loop\n");
+   dev_dbg(>dev,
+   "Indicator LED fault: Short circuit or open loop\n");
 
dev_dbg(>dev, "%u connected LEDs\n",
rval & AS_FAULT_INFO_LED_AMOUNT ? 2 : 1);
@@ -315,8 +315,8 @@ static int as3645a_read_fault(struct as3645a *flash)
dev_dbg(>dev, "Short circuit fault\n");
 
if (rval & AS_FAULT_INFO_OVER_VOLTAGE)
-   dev_dbg(>dev, "Over voltage fault: "
-   "Indicates missing capacitor or open connection\n");
+   dev_dbg(>dev,
+   "Over voltage fault: Indicates missing capacitor or 
open connection\n");
 
return rval;
 }
@@ -588,8 +588,9 @@ static int as3645a_registered(struct v4l2_subdev *sd)
 
/* Verify the chip model and version. */
if (model != 0x01 || rfu != 0x00) {
-   dev_err(>dev, "AS3645A not detected "
-   "(model %d rfu %d)\n", model, rfu);
+   dev_err(>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..2f72bba64d99 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -775,8 +775,8 @@ 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 :