| Message ID | 20180107150801.21617-1-gebulmer@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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, -- 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
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, > > -- 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
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 -- 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
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 -- 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
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 -- 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
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 -- 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
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,
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(-)