Skip to contentSkip to conversation

Staging: iio: adc: Checkpatch.pl warning cleanups

PATCH Superseded Brian Vandre 7 participants 16 messages Oct 21, 2014 – Oct 23, 2014

Subsystems (4)
Arm i.MX and Layerscape Staging IIO Staging - industrial io
lore
Share & tools
lore
author
#3 lore raw

PATCH 2/2 Staging: iio: adc: fix line over 80 characters

In reply to Brian Vandre

This fixes the checkpatch.pl warning:
WARNING: line over 80 characters

Signed-off-by Brian Vandre <[email protected]>
---
 drivers/staging/iio/adc/mxs-lradc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/iio/adc/mxs-lradc.c +2 -1
index 7d4b068..62449a6 100644
@@ -514,7 +514,8 @@ static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay), LRADC_DELAY(2));
Signature · 1 line
1.9.1

Brian Vandre

author
#0 lore raw

cover Staging: iio: adc: Checkpatch.pl warning cleanups

This patch-set fixes 2 checkpatch.pl warnings
dealing with lines over 80 characters.

Brian Vandre (2):
Staging: iio: adc: fix line over 80 characters
Staging: iio: adc: fix line over 80 characters

 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/2 Staging: iio: adc: fix line over 80 characters

This fixes the checkpatch.pl warning:
WARNING: line over 80 characters

Signed-off-by Brian Vandre <[email protected]>
---
 drivers/staging/iio/adc/mxs-lradc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/iio/adc/mxs-lradc.c +2 -1
index 32a1926..7d4b068 100644
@@ -455,7 +455,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay),
Signature · 1 line
1.9.1
#4 lore raw
In reply to Brian Vandre

Hi,

These are two patches with the same commit log, in the same file. You can probably squash them.

Quoted 13 lines · Brian Vandre

This patch-set fixes 2 checkpatch.pl warnings
dealing with lines over 80 characters.

Brian Vandre (2):
Staging: iio: adc: fix line over 80 characters
Staging: iio: adc: fix line over 80 characters

 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Signature · 1 line
1.9.1
Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#6 lore raw

PATCH v2 1/1 Staging: iio: adc: fix line over 80 characters

In reply to Brian Vandre

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

Signed-off-by Brian Vandre <[email protected]>
---
 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/iio/adc/mxs-lradc.c +4 -2
index 32a1926..62449a6 100644
@@ -455,7 +455,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay),

@@ -513,7 +514,8 @@ static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay), LRADC_DELAY(2));
Signature · 1 line
1.9.1
author
#5 lore raw

PATCH v2 1/1 Staging: iio: adc: Checkpatch.pl warning cleanups

In reply to Brian Vandre

This patch-set fixes 2 checkpatch.pl warnings
dealing with lines over 80 characters.

Changes from v1:
-squashed 2 patches into 1 as suggested by Alexandre Belloni

Brian Vandre (1):
Staging: iio: adc: fix line over 80 characters

 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Signature · 1 line
1.9.1
#7 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

In reply to Brian Vandre
Quoted 3 lines · Brian Vandre

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

thanks
sudip

Quoted 37 lines
Signed-off-by Brian Vandre <[email protected]>
---
 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/iio/adc/mxs-lradc.c +4 -2
index 32a1926..62449a6 100644
@@ -455,7 +455,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay),

@@ -513,7 +514,8 @@ static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay), LRADC_DELAY(2));
Signature · 7 lines
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
#8 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

Sudip Mukherjee schrieb am 22.10.2014 06:21:

Quoted 7 lines · Brian Vandre
Quoted 3 lines

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

Good point, but what solution would you propose?

Quoted 45 lines

thanks
sudip

Quoted 37 lines
Signed-off-by Brian Vandre <[email protected]>
---
 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/iio/adc/mxs-lradc.c +4 -2
index 32a1926..62449a6 100644
@@ -455,7 +455,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay),

@@ -513,7 +514,8 @@ static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay), LRADC_DELAY(2));
Signature · 7 lines
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#9 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

Quoted 9 lines · Hartmut Knaack

Sudip Mukherjee schrieb am 22.10.2014 06:21:

Quoted 7 lines · Brian Vandre
Quoted 3 lines

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

Good point, but what solution would you propose?

Hey All,

Thanks for all the feedback. This is my first attempt at a patch so I thank you all for helping me through it. I have a question about the strict option on checkpatch.pl. I thought that the stict option was not necessarily part of the coding standards but more of a nice to have. Should I be always using the strict option?

On this particular patch if I were to align to the open parenthesis it would push the comment "/* trigger DELAY unit#3 */" off to the next line which I thought we be less clear. If --strict is optional then I would argue to leave it the way it is, but again this is my first time and I am learning.

If I were to align to the parenthesis should I just move the comment to the next line or possibly delete the comment altogether? The code is clear and the comment might not be necessary but I didn't want to remove anything the original author wrote.

Thanks,
Brian Vandre

#10 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

In reply to Hartmut Knaack
Quoted 12 lines · Hartmut Knaack

Sudip Mukherjee schrieb am 22.10.2014 06:21:

Quoted 7 lines · Brian Vandre
Quoted 3 lines

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

Good point, but what solution would you propose?

Quoted 1 line
Quoted 1 line
drivers/staging/iio/adc/mxs-lradc.c +0 -0
Quoted 10 lines
Quoted 10 lines

drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch) · line 462

Quoted 10 lines
drivers/staging/iio/adc/mxs-lradc.c +2 -1
@@ -455,7 +455,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay),

Maybe something like:

	mxs_lradc_reg_wrt(lradc,
			  LRADC_DELAY_TRIGGER(0) |
						/* don't trigger ADC */
			  LRADC_DELAY_TRIGGER_DELAYS(1 << 3) |
						/* trigger DELAY unit#3 */
			  LRADC_DELAY_KICK |
			  LRADC_DELAY_DELAY(lradc->settling_delay),
#11 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

In reply to Hartmut Knaack
Quoted 9 lines · Hartmut Knaack

Sudip Mukherjee schrieb am 22.10.2014 06:21:

Quoted 7 lines · Brian Vandre
Quoted 3 lines

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

Good point, but what solution would you propose?

when you are breaking the line , make a point to align the next line with the open parenthesis. like :
- mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+ mxs_lradc_reg_wrt(lradc,
+ LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */

I have seen in many patches Greg commenting to resend the patch after aligning it.
so my opinion , even though it is optional , better to use it while sending a patch , so that no one can say about that.

happened to me in few of my initial patches. :)

Quoted 46 lines
Quoted 45 lines

thanks
sudip

Quoted 37 lines
Signed-off-by Brian Vandre <[email protected]>
---
 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/iio/adc/mxs-lradc.c +4 -2
index 32a1926..62449a6 100644
@@ -455,7 +455,8 @@ static void mxs_lradc_setup_ts_channel(struct mxs_lradc *lradc, unsigned ch)
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay),

@@ -513,7 +514,8 @@ static void mxs_lradc_setup_ts_pressure(struct mxs_lradc *lradc, unsigned ch1,
 	 * SoC's delay unit and start the conversion later
 	 * and automatically.
 	 */
-	mxs_lradc_reg_wrt(lradc, LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
+	mxs_lradc_reg_wrt(lradc,
+		LRADC_DELAY_TRIGGER(0) | /* don't trigger ADC */
 		LRADC_DELAY_TRIGGER_DELAYS(1 << 3) | /* trigger DELAY unit#3 */
 		LRADC_DELAY_KICK |
 		LRADC_DELAY_DELAY(lradc->settling_delay), LRADC_DELAY(2));
Signature · 7 lines
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw

Re: [PATCH 1/2] Staging: iio: adc: fix line over 80 characters

In reply to Brian Vandre
Quoted 4 lines · Brian Vandre

This fixes the checkpatch.pl warning:
WARNING: line over 80 characters

Signed-off-by Brian Vandre <[email protected]>

As a part of OPW [1] IIO cleanup project [2] we analyzed all checkpatch.pl warnings / errors and we decided not to fix some of them as there is no obvious improvement.

This "line over 80 characters" warning was one of them. In my opinion it doesn't make code easier to read.

thanks,
Daniel.

[1] http://kernelnewbies.org/OPWIntro
[2] http://kernelnewbies.org/IIO_cleanup

#12 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

In reply to Sudip Mukherjee
Quoted 6 lines · Sudip Mukherjee
Quoted 3 lines · Brian Vandre

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

Those checkpatch warnings are suggestions, not hard requirements. The idea is to improve code legibility, but if the change has the adverse effect the warning can and should be ignored. Also when making a change you should keep the existing indention style of a file.

- Lars

author
#13 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

Quoted 8 lines · Lars-Peter Clausen
Quoted 6 lines · Sudip Mukherjee
Quoted 3 lines · Brian Vandre

This fixes the 2 checkpatch.pl warnings:
WARNING: line over 80 characters

please check your patch with --strict option of checkpatch.pl , and you will get :
"Alignment should match open parenthesis" .

Quoted 7 lines

Those checkpatch warnings are suggestions, not hard requirements. The idea is to improve code legibility, but if the change has the adverse effect the warning can and should be ignored. Also when making a change you should keep the existing indention style of a file.

- Lars

I think my patch does follow the style of the original file. If you run checkpatch.pl --strict on the whole file you will get many open parenthesis warnings. I believe it does make it slightly more legibile.

author
#14 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

In reply to Brian Vandre

With all the replies I have gotten it seems like there might not be a good path forward with this patch. I am starting to agree with what Daniel Baluta said above that this doesn't make the code easier to read. All the other suggestions don't quite fit the same style as the rest of the file so it might just be better to leave it.

This being my first try I thank you all for your input. It has helped me learn quite a bit.

Hopefully on the next patch I can fix something a little more meaningful!

Thanks,
Brian Vandre

#15 lore raw

Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters

Hi,

Quoted 9 lines · Brian Vandre

With all the replies I have gotten it seems like there might not be a good path forward with this patch. I am starting to agree with what Daniel Baluta said above that this doesn't make the code easier to read. All the other suggestions don't quite fit the same style as the rest of the file so it might just be better to leave it.

This being my first try I thank you all for your input. It has helped me learn quite a bit.

Hopefully on the next patch I can fix something a little more meaningful!

Thank you for your effort anyway. My last comment would be that you don't need a cover letter when sending only one patch.

There are plenty of things to fix in the kernel, maybe we can help you find something.

Signature · 3 lines
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com