[PATCH] staging: iio: light: Add breaks to lengthy lines

[PATCH] staging: iio: light: Add breaks to lengthy lines

[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

Re: [PATCH] staging: iio: light: Add breaks to lengthy lines

[Jonathan Cameron]

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,

Re: [PATCH] staging: iio: light: Add breaks to lengthy lines

[Joe Perches]

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,
> 
> 

Re: [PATCH] staging: iio: light: Add breaks to lengthy lines

[Jonathan Cameron]

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

Re: [PATCH] staging: iio: light: Add breaks to lengthy lines

[Dan Carpenter]

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

Re: [PATCH] staging: iio: light: Add breaks to lengthy lines

[Joe Perches]

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

Re: [PATCH] staging: iio: light: Add breaks to lengthy lines

[Dan Carpenter]

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