linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: iio: light: Add breaks to lengthy lines
@ 2018-01-07 15:08 George Edward Bulmer
  2018-01-07 16:28 ` Jonathan Cameron
  2018-01-08 11:48 ` Dan Carpenter
  0 siblings, 2 replies; 7+ messages in thread
From: George Edward Bulmer @ 2018-01-07 15:08 UTC (permalink / raw)
  To: gregkh
  Cc: jic23, knaack.h, lars, pmeerw, masneyb, linux-iio, devel,
	linux-kernel, George Edward Bulmer

This fixes three instances of checkpatch warning:
WARNING: line over 80 characters

Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>
---
 drivers/staging/iio/light/tsl2x7x.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
index 42ed9c015aaf..126e11530ce0 100644
--- a/drivers/staging/iio/light/tsl2x7x.c
+++ b/drivers/staging/iio/light/tsl2x7x.c
@@ -1441,7 +1441,8 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
 
 static struct attribute *tsl2x7x_ALS_device_attrs[] = {
 	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance0_integration_time_available
+		.dev_attr.attr,
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
 	&dev_attr_in_illuminance0_lux_table.attr,
@@ -1455,7 +1456,8 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
 
 static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
 	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance0_integration_time_available
+		.dev_attr.attr,
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
 	&dev_attr_in_illuminance0_lux_table.attr,
@@ -1471,7 +1473,8 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
 
 static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
 	&dev_attr_in_illuminance0_calibscale_available.attr,
-	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
+	&iio_const_attr_in_illuminance0_integration_time_available
+		.dev_attr.attr,
 	&dev_attr_in_illuminance0_target_input.attr,
 	&dev_attr_in_illuminance0_calibrate.attr,
 	&dev_attr_in_illuminance0_lux_table.attr,
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] staging: iio: light: Add breaks to lengthy lines
  2018-01-07 15:08 [PATCH] staging: iio: light: Add breaks to lengthy lines George Edward Bulmer
@ 2018-01-07 16:28 ` Jonathan Cameron
  2018-01-07 16:42   ` Joe Perches
  2018-01-08 11:48 ` Dan Carpenter
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2018-01-07 16:28 UTC (permalink / raw)
  To: George Edward Bulmer
  Cc: gregkh, knaack.h, lars, pmeerw, masneyb, linux-iio, devel, linux-kernel

On Sun,  7 Jan 2018 15:08:01 +0000
George Edward Bulmer <gebulmer@googlemail.com> wrote:

> This fixes three instances of checkpatch warning:
> WARNING: line over 80 characters
> 
> Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>
Applied, thanks,

Jonathan
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 42ed9c015aaf..126e11530ce0 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1441,7 +1441,8 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  
>  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance0_integration_time_available
> +		.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
>  	&dev_attr_in_illuminance0_lux_table.attr,
> @@ -1455,7 +1456,8 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance0_integration_time_available
> +		.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
>  	&dev_attr_in_illuminance0_lux_table.attr,
> @@ -1471,7 +1473,8 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
>  
>  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance0_integration_time_available
> +		.dev_attr.attr,
>  	&dev_attr_in_illuminance0_target_input.attr,
>  	&dev_attr_in_illuminance0_calibrate.attr,
>  	&dev_attr_in_illuminance0_lux_table.attr,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] staging: iio: light: Add breaks to lengthy lines
  2018-01-07 16:28 ` Jonathan Cameron
@ 2018-01-07 16:42   ` Joe Perches
  2018-01-07 17:17     ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Joe Perches @ 2018-01-07 16:42 UTC (permalink / raw)
  To: Jonathan Cameron, George Edward Bulmer
  Cc: gregkh, knaack.h, lars, pmeerw, masneyb, linux-iio, devel, linux-kernel

On Sun, 2018-01-07 at 16:28 +0000, Jonathan Cameron wrote:
> On Sun,  7 Jan 2018 15:08:01 +0000
> George Edward Bulmer <gebulmer@googlemail.com> wrote:
> 
> > This fixes three instances of checkpatch warning:
> > WARNING: line over 80 characters
> > 
> > Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>
> 
> Applied, thanks,

I believe these are examples of long line conversions
that should not be applied.

Most of the problem here is the very long identifier naming.
57 character identifiers are pretty unnecessary.

> Jonathan
> > ---
> >  drivers/staging/iio/light/tsl2x7x.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> > index 42ed9c015aaf..126e11530ce0 100644
> > --- a/drivers/staging/iio/light/tsl2x7x.c
> > +++ b/drivers/staging/iio/light/tsl2x7x.c
> > @@ -1441,7 +1441,8 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
> >  
> >  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
> >  	&dev_attr_in_illuminance0_calibscale_available.attr,
> > -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> > +	&iio_const_attr_in_illuminance0_integration_time_available
> > +		.dev_attr.attr,
> >  	&dev_attr_in_illuminance0_target_input.attr,
> >  	&dev_attr_in_illuminance0_calibrate.attr,
> >  	&dev_attr_in_illuminance0_lux_table.attr,
> > @@ -1455,7 +1456,8 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
> >  
> >  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
> >  	&dev_attr_in_illuminance0_calibscale_available.attr,
> > -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> > +	&iio_const_attr_in_illuminance0_integration_time_available
> > +		.dev_attr.attr,
> >  	&dev_attr_in_illuminance0_target_input.attr,
> >  	&dev_attr_in_illuminance0_calibrate.attr,
> >  	&dev_attr_in_illuminance0_lux_table.attr,
> > @@ -1471,7 +1473,8 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
> >  
> >  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
> >  	&dev_attr_in_illuminance0_calibscale_available.attr,
> > -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> > +	&iio_const_attr_in_illuminance0_integration_time_available
> > +		.dev_attr.attr,
> >  	&dev_attr_in_illuminance0_target_input.attr,
> >  	&dev_attr_in_illuminance0_calibrate.attr,
> >  	&dev_attr_in_illuminance0_lux_table.attr,
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] staging: iio: light: Add breaks to lengthy lines
  2018-01-07 16:42   ` Joe Perches
@ 2018-01-07 17:17     ` Jonathan Cameron
  2018-01-08 12:02       ` Dan Carpenter
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2018-01-07 17:17 UTC (permalink / raw)
  To: Joe Perches
  Cc: George Edward Bulmer, gregkh, knaack.h, lars, pmeerw, masneyb,
	linux-iio, devel, linux-kernel

On Sun, 07 Jan 2018 08:42:27 -0800
Joe Perches <joe@perches.com> wrote:

> On Sun, 2018-01-07 at 16:28 +0000, Jonathan Cameron wrote:
> > On Sun,  7 Jan 2018 15:08:01 +0000
> > George Edward Bulmer <gebulmer@googlemail.com> wrote:
> >   
> > > This fixes three instances of checkpatch warning:
> > > WARNING: line over 80 characters
> > > 
> > > Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>  
> > 
> > Applied, thanks,  
> 
> I believe these are examples of long line conversions
> that should not be applied.

On this one I disagree. The line breaks don't hurt readability
so aren't a problem.  The benefit is admittedly small, however
we are adding churn in a fairly safe place - this stuff rarely
changes.

> 
> Most of the problem here is the very long identifier naming.
> 57 character identifiers are pretty unnecessary.

The long line is because the name is encoding a very specific thing
and personally I would rather not have to guess what a shorter abbreviation
meant for the cost of saving a few characters in a filename.

Of course, we could deliberately use a different name for the sysfs
file from the value used for the identifier in the code, but that
would result in complexity just to reduce the line length.

This particular one is close to the most pathological case in the IIO
ABI though :)

It is possible to change to the code constructed version of the same
thing, but that's a rather more involved change and one we haven't
commonly done.  (using the .read_avail callbacks in IIO).

Jonathan

> 
> > Jonathan  
> > > ---
> > >  drivers/staging/iio/light/tsl2x7x.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> > > index 42ed9c015aaf..126e11530ce0 100644
> > > --- a/drivers/staging/iio/light/tsl2x7x.c
> > > +++ b/drivers/staging/iio/light/tsl2x7x.c
> > > @@ -1441,7 +1441,8 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
> > >  
> > >  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
> > >  	&dev_attr_in_illuminance0_calibscale_available.attr,
> > > -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> > > +	&iio_const_attr_in_illuminance0_integration_time_available
> > > +		.dev_attr.attr,
> > >  	&dev_attr_in_illuminance0_target_input.attr,
> > >  	&dev_attr_in_illuminance0_calibrate.attr,
> > >  	&dev_attr_in_illuminance0_lux_table.attr,
> > > @@ -1455,7 +1456,8 @@ static struct attribute *tsl2x7x_PRX_device_attrs[] = {
> > >  
> > >  static struct attribute *tsl2x7x_ALSPRX_device_attrs[] = {
> > >  	&dev_attr_in_illuminance0_calibscale_available.attr,
> > > -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> > > +	&iio_const_attr_in_illuminance0_integration_time_available
> > > +		.dev_attr.attr,
> > >  	&dev_attr_in_illuminance0_target_input.attr,
> > >  	&dev_attr_in_illuminance0_calibrate.attr,
> > >  	&dev_attr_in_illuminance0_lux_table.attr,
> > > @@ -1471,7 +1473,8 @@ static struct attribute *tsl2x7x_PRX2_device_attrs[] = {
> > >  
> > >  static struct attribute *tsl2x7x_ALSPRX2_device_attrs[] = {
> > >  	&dev_attr_in_illuminance0_calibscale_available.attr,
> > > -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> > > +	&iio_const_attr_in_illuminance0_integration_time_available
> > > +		.dev_attr.attr,
> > >  	&dev_attr_in_illuminance0_target_input.attr,
> > >  	&dev_attr_in_illuminance0_calibrate.attr,
> > >  	&dev_attr_in_illuminance0_lux_table.attr,  
> > 
> >   
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] staging: iio: light: Add breaks to lengthy lines
  2018-01-07 15:08 [PATCH] staging: iio: light: Add breaks to lengthy lines George Edward Bulmer
  2018-01-07 16:28 ` Jonathan Cameron
@ 2018-01-08 11:48 ` Dan Carpenter
  1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2018-01-08 11:48 UTC (permalink / raw)
  To: George Edward Bulmer
  Cc: gregkh, devel, lars, linux-iio, linux-kernel, pmeerw, knaack.h, jic23

On Sun, Jan 07, 2018 at 03:08:01PM +0000, George Edward Bulmer wrote:
> This fixes three instances of checkpatch warning:
> WARNING: line over 80 characters
> 
> Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>
> ---
>  drivers/staging/iio/light/tsl2x7x.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/iio/light/tsl2x7x.c b/drivers/staging/iio/light/tsl2x7x.c
> index 42ed9c015aaf..126e11530ce0 100644
> --- a/drivers/staging/iio/light/tsl2x7x.c
> +++ b/drivers/staging/iio/light/tsl2x7x.c
> @@ -1441,7 +1441,8 @@ static irqreturn_t tsl2x7x_event_handler(int irq, void *private)
>  
>  static struct attribute *tsl2x7x_ALS_device_attrs[] = {
>  	&dev_attr_in_illuminance0_calibscale_available.attr,
> -	&iio_const_attr_in_illuminance0_integration_time_available.dev_attr.attr,
> +	&iio_const_attr_in_illuminance0_integration_time_available
> +		.dev_attr.attr,

Ugh.  No.  The original was better.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] staging: iio: light: Add breaks to lengthy lines
  2018-01-07 17:17     ` Jonathan Cameron
@ 2018-01-08 12:02       ` Dan Carpenter
  2018-01-08 14:59         ` Joe Perches
  0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2018-01-08 12:02 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Joe Perches, devel, lars, linux-iio, gregkh, linux-kernel,
	pmeerw, knaack.h, George Edward Bulmer

On Sun, Jan 07, 2018 at 05:17:42PM +0000, Jonathan Cameron wrote:
> On Sun, 07 Jan 2018 08:42:27 -0800
> Joe Perches <joe@perches.com> wrote:
> 
> > On Sun, 2018-01-07 at 16:28 +0000, Jonathan Cameron wrote:
> > > On Sun,  7 Jan 2018 15:08:01 +0000
> > > George Edward Bulmer <gebulmer@googlemail.com> wrote:
> > >   
> > > > This fixes three instances of checkpatch warning:
> > > > WARNING: line over 80 characters
> > > > 
> > > > Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>  
> > > 
> > > Applied, thanks,  
> > 
> > I believe these are examples of long line conversions
> > that should not be applied.
> 
> On this one I disagree. The line breaks don't hurt readability
> so aren't a problem.  The benefit is admittedly small, however
> we are adding churn in a fairly safe place - this stuff rarely
> changes.

They do hurt read
ability.

Checkpatch.pl should maybe have a special case to not complain if the
last variable in the line is over 40 characters long.  I didn't
realize that we had LONG_LINE_COMMENT.  Does that mean we don't complain
if comment goes over 80 characters?

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] staging: iio: light: Add breaks to lengthy lines
  2018-01-08 12:02       ` Dan Carpenter
@ 2018-01-08 14:59         ` Joe Perches
  0 siblings, 0 replies; 7+ messages in thread
From: Joe Perches @ 2018-01-08 14:59 UTC (permalink / raw)
  To: Dan Carpenter, Jonathan Cameron
  Cc: devel, lars, linux-iio, gregkh, linux-kernel, pmeerw, knaack.h,
	George Edward Bulmer

On Mon, 2018-01-08 at 15:02 +0300, Dan Carpenter wrote:
> On Sun, Jan 07, 2018 at 05:17:42PM +0000, Jonathan Cameron wrote:
> > On Sun, 07 Jan 2018 08:42:27 -0800
> > Joe Perches <joe@perches.com> wrote:
> > 
> > > On Sun, 2018-01-07 at 16:28 +0000, Jonathan Cameron wrote:
> > > > On Sun,  7 Jan 2018 15:08:01 +0000
> > > > George Edward Bulmer <gebulmer@googlemail.com> wrote:
> > > >   
> > > > > This fixes three instances of checkpatch warning:
> > > > > WARNING: line over 80 characters
> > > > > 
> > > > > Signed-off-by: George Edward Bulmer <gebulmer@googlemail.com>  
> > > > 
> > > > Applied, thanks,  
> > > 
> > > I believe these are examples of long line conversions
> > > that should not be applied.
> > 
> > On this one I disagree. The line breaks don't hurt readability
> > so aren't a problem.  The benefit is admittedly small, however
> > we are adding churn in a fairly safe place - this stuff rarely
> > changes.
> 
> They do hurt read
> ability.
> 
> Checkpatch.pl should maybe have a special case to not complain if the
> last variable in the line is over 40 characters long.  I didn't
> realize that we had LONG_LINE_COMMENT.  Does that mean we don't complain
> if comment goes over 80 characters?

No.  It's just an extra classifier.

It means that long lines with comments that
exceed 80 columns can be ignored by using the
command line argument

--ignore=long_line_comment

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-08 14:59 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-07 15:08 [PATCH] staging: iio: light: Add breaks to lengthy lines George Edward Bulmer
2018-01-07 16:28 ` Jonathan Cameron
2018-01-07 16:42   ` Joe Perches
2018-01-07 17:17     ` Jonathan Cameron
2018-01-08 12:02       ` Dan Carpenter
2018-01-08 14:59         ` Joe Perches
2018-01-08 11:48 ` Dan Carpenter

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).