All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups
@ 2014-10-21  2:48 Brian Vandre
  2014-10-21  2:48 ` [PATCH 1/2] Staging: iio: adc: fix line over 80 characters Brian Vandre
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Brian Vandre @ 2014-10-21  2:48 UTC (permalink / raw)
  To: gregkh, jic23, alexandre.belloni, linux-iio; +Cc: Brian Vandre, linux-kernel

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

-- 
1.9.1


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

* [PATCH 1/2] Staging: iio: adc: fix line over 80 characters
  2014-10-21  2:48 [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Brian Vandre
@ 2014-10-21  2:48 ` Brian Vandre
  2014-10-23 10:39   ` Daniel Baluta
  2014-10-21  2:48 ` [PATCH 2/2] " Brian Vandre
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Brian Vandre @ 2014-10-21  2:48 UTC (permalink / raw)
  To: gregkh, jic23, alexandre.belloni, linux-iio; +Cc: Brian Vandre, linux-kernel

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

Signed-off-by: Brian Vandre <bvandre@gmail.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 32a1926..7d4b068 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -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),
-- 
1.9.1


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

* [PATCH 2/2] Staging: iio: adc: fix line over 80 characters
  2014-10-21  2:48 [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Brian Vandre
  2014-10-21  2:48 ` [PATCH 1/2] Staging: iio: adc: fix line over 80 characters Brian Vandre
@ 2014-10-21  2:48 ` Brian Vandre
  2014-10-21  7:29 ` [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Alexandre Belloni
  2014-10-21 22:56 ` [PATCH v2] " Brian Vandre
  3 siblings, 0 replies; 17+ messages in thread
From: Brian Vandre @ 2014-10-21  2:48 UTC (permalink / raw)
  To: gregkh, jic23, alexandre.belloni, linux-iio; +Cc: Brian Vandre, linux-kernel

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

Signed-off-by: Brian Vandre <bvandre@gmail.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 7d4b068..62449a6 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -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));
-- 
1.9.1


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

* Re: [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups
  2014-10-21  2:48 [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Brian Vandre
  2014-10-21  2:48 ` [PATCH 1/2] Staging: iio: adc: fix line over 80 characters Brian Vandre
  2014-10-21  2:48 ` [PATCH 2/2] " Brian Vandre
@ 2014-10-21  7:29 ` Alexandre Belloni
  2014-10-21 22:56 ` [PATCH v2] " Brian Vandre
  3 siblings, 0 replies; 17+ messages in thread
From: Alexandre Belloni @ 2014-10-21  7:29 UTC (permalink / raw)
  To: Brian Vandre; +Cc: gregkh, jic23, linux-iio, linux-kernel

Hi,

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

On 20/10/2014 at 21:48:47 -0500, Brian Vandre wrote :
> 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(-)
> 
> -- 
> 1.9.1
> 

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

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

* [PATCH v2] Staging: iio: adc: Checkpatch.pl warning cleanups
  2014-10-21  2:48 [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Brian Vandre
                   ` (2 preceding siblings ...)
  2014-10-21  7:29 ` [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Alexandre Belloni
@ 2014-10-21 22:56 ` Brian Vandre
  2014-10-21 22:56   ` [PATCH v2] Staging: iio: adc: fix line over 80 characters Brian Vandre
  3 siblings, 1 reply; 17+ messages in thread
From: Brian Vandre @ 2014-10-21 22:56 UTC (permalink / raw)
  To: gregkh, jic23, alexandre.belloni, linux-iio, marex
  Cc: Brian Vandre, linux-kernel

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

-- 
1.9.1


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

* [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-21 22:56 ` [PATCH v2] " Brian Vandre
@ 2014-10-21 22:56   ` Brian Vandre
  2014-10-22  4:21     ` Sudip Mukherjee
  2014-10-23 12:53     ` Brian Vandre
  0 siblings, 2 replies; 17+ messages in thread
From: Brian Vandre @ 2014-10-21 22:56 UTC (permalink / raw)
  To: gregkh, jic23, alexandre.belloni, linux-iio, marex
  Cc: Brian Vandre, linux-kernel

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

Signed-off-by: Brian Vandre <bvandre@gmail.com>
---
 drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
index 32a1926..62449a6 100644
--- a/drivers/staging/iio/adc/mxs-lradc.c
+++ b/drivers/staging/iio/adc/mxs-lradc.c
@@ -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));
-- 
1.9.1


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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-21 22:56   ` [PATCH v2] Staging: iio: adc: fix line over 80 characters Brian Vandre
@ 2014-10-22  4:21     ` Sudip Mukherjee
  2014-10-22 23:47       ` Hartmut Knaack
  2014-10-23 10:50       ` Lars-Peter Clausen
  2014-10-23 12:53     ` Brian Vandre
  1 sibling, 2 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2014-10-22  4:21 UTC (permalink / raw)
  To: Brian Vandre
  Cc: gregkh, jic23, alexandre.belloni, linux-iio, marex, linux-kernel

On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
> 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

> Signed-off-by: Brian Vandre <bvandre@gmail.com>
> ---
>  drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> index 32a1926..62449a6 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -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));
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-22  4:21     ` Sudip Mukherjee
@ 2014-10-22 23:47       ` Hartmut Knaack
  2014-10-23  2:02         ` Brian Vandre
                           ` (2 more replies)
  2014-10-23 10:50       ` Lars-Peter Clausen
  1 sibling, 3 replies; 17+ messages in thread
From: Hartmut Knaack @ 2014-10-22 23:47 UTC (permalink / raw)
  To: Sudip Mukherjee, Brian Vandre
  Cc: gregkh, jic23, alexandre.belloni, linux-iio, marex, linux-kernel

Sudip Mukherjee schrieb am 22.10.2014 06:21:
> On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
>> 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?
> thanks
> sudip
> 
>> Signed-off-by: Brian Vandre <bvandre@gmail.com>
>> ---
>>  drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
>> index 32a1926..62449a6 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -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));
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-22 23:47       ` Hartmut Knaack
@ 2014-10-23  2:02         ` Brian Vandre
  2014-10-23  3:23           ` Joe Perches
  2014-10-23  9:46         ` Sudip Mukherjee
  2 siblings, 0 replies; 17+ messages in thread
From: Brian Vandre @ 2014-10-23  2:02 UTC (permalink / raw)
  To: Hartmut Knaack
  Cc: Sudip Mukherjee, gregkh, jic23, alexandre.belloni, linux-iio,
	marex, linux-kernel

On Thu, Oct 23, 2014 at 01:47:37AM +0200, Hartmut Knaack wrote:
> Sudip Mukherjee schrieb am 22.10.2014 06:21:
> > On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
> >> 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

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-22 23:47       ` Hartmut Knaack
@ 2014-10-23  3:23           ` Joe Perches
  2014-10-23  3:23           ` Joe Perches
  2014-10-23  9:46         ` Sudip Mukherjee
  2 siblings, 0 replies; 17+ messages in thread
From: Joe Perches @ 2014-10-23  3:23 UTC (permalink / raw)
  To: Hartmut Knaack
  Cc: Sudip Mukherjee, Brian Vandre, gregkh, jic23, alexandre.belloni,
	linux-iio, marex, linux-kernel

On Thu, 2014-10-23 at 01:47 +0200, Hartmut Knaack wrote:
> Sudip Mukherjee schrieb am 22.10.2014 06:21:
> > On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
> >> 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?

> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c

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



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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
@ 2014-10-23  3:23           ` Joe Perches
  0 siblings, 0 replies; 17+ messages in thread
From: Joe Perches @ 2014-10-23  3:23 UTC (permalink / raw)
  To: Hartmut Knaack
  Cc: Sudip Mukherjee, Brian Vandre, gregkh, jic23, alexandre.belloni,
	linux-iio, marex, linux-kernel

On Thu, 2014-10-23 at 01:47 +0200, Hartmut Knaack wrote:
> Sudip Mukherjee schrieb am 22.10.2014 06:21:
> > On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
> >> 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?

> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c

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

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-22 23:47       ` Hartmut Knaack
  2014-10-23  2:02         ` Brian Vandre
  2014-10-23  3:23           ` Joe Perches
@ 2014-10-23  9:46         ` Sudip Mukherjee
  2 siblings, 0 replies; 17+ messages in thread
From: Sudip Mukherjee @ 2014-10-23  9:46 UTC (permalink / raw)
  To: Hartmut Knaack
  Cc: Brian Vandre, gregkh, jic23, alexandre.belloni, linux-iio, marex,
	linux-kernel

On Thu, Oct 23, 2014 at 01:47:37AM +0200, Hartmut Knaack wrote:
> Sudip Mukherjee schrieb am 22.10.2014 06:21:
> > On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
> >> 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. :) 


> > thanks
> > sudip
> > 
> >> Signed-off-by: Brian Vandre <bvandre@gmail.com>
> >> ---
> >>  drivers/staging/iio/adc/mxs-lradc.c | 6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c
> >> index 32a1926..62449a6 100644
> >> --- a/drivers/staging/iio/adc/mxs-lradc.c
> >> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> >> @@ -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));
> >> -- 
> >> 1.9.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> >> the body of a message to majordomo@vger.kernel.org
> >> 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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

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

* Re: [PATCH 1/2] Staging: iio: adc: fix line over 80 characters
  2014-10-21  2:48 ` [PATCH 1/2] Staging: iio: adc: fix line over 80 characters Brian Vandre
@ 2014-10-23 10:39   ` Daniel Baluta
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Baluta @ 2014-10-23 10:39 UTC (permalink / raw)
  To: Brian Vandre
  Cc: Greg Kroah-Hartman, Jonathan Cameron, alexandre.belloni,
	linux-iio, Linux Kernel Mailing List

On Tue, Oct 21, 2014 at 5:48 AM, Brian Vandre <bvandre@gmail.com> wrote:
> This fixes the checkpatch.pl warning:
> WARNING: line over 80 characters
>
> Signed-off-by: Brian Vandre <bvandre@gmail.com>

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

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-22  4:21     ` Sudip Mukherjee
  2014-10-22 23:47       ` Hartmut Knaack
@ 2014-10-23 10:50       ` Lars-Peter Clausen
  2014-10-23 12:42         ` Brian Vandre
  1 sibling, 1 reply; 17+ messages in thread
From: Lars-Peter Clausen @ 2014-10-23 10:50 UTC (permalink / raw)
  To: Sudip Mukherjee, Brian Vandre
  Cc: gregkh, jic23, alexandre.belloni, linux-iio, marex, linux-kernel

On 10/22/2014 06:21 AM, Sudip Mukherjee wrote:
> On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
>> 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

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-23 10:50       ` Lars-Peter Clausen
@ 2014-10-23 12:42         ` Brian Vandre
  0 siblings, 0 replies; 17+ messages in thread
From: Brian Vandre @ 2014-10-23 12:42 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Sudip Mukherjee, gregkh, jic23, alexandre.belloni, linux-iio,
	marex, linux-kernel

On Thu, Oct 23, 2014 at 12:50:00PM +0200, Lars-Peter Clausen wrote:
> On 10/22/2014 06:21 AM, Sudip Mukherjee wrote:
> >On Tue, Oct 21, 2014 at 05:56:47PM -0500, Brian Vandre wrote:
> >>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

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.

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-21 22:56   ` [PATCH v2] Staging: iio: adc: fix line over 80 characters Brian Vandre
  2014-10-22  4:21     ` Sudip Mukherjee
@ 2014-10-23 12:53     ` Brian Vandre
  2014-10-23 13:27       ` Alexandre Belloni
  1 sibling, 1 reply; 17+ messages in thread
From: Brian Vandre @ 2014-10-23 12:53 UTC (permalink / raw)
  To: gregkh, jic23, alexandre.belloni, linux-iio, marex; +Cc: linux-kernel

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

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

* Re: [PATCH v2] Staging: iio: adc: fix line over 80 characters
  2014-10-23 12:53     ` Brian Vandre
@ 2014-10-23 13:27       ` Alexandre Belloni
  0 siblings, 0 replies; 17+ messages in thread
From: Alexandre Belloni @ 2014-10-23 13:27 UTC (permalink / raw)
  To: Brian Vandre; +Cc: gregkh, jic23, linux-iio, marex, linux-kernel

Hi,

On 23/10/2014 at 07:53:36 -0500, Brian Vandre wrote :
> 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.

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

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

end of thread, other threads:[~2014-10-23 13:27 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21  2:48 [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Brian Vandre
2014-10-21  2:48 ` [PATCH 1/2] Staging: iio: adc: fix line over 80 characters Brian Vandre
2014-10-23 10:39   ` Daniel Baluta
2014-10-21  2:48 ` [PATCH 2/2] " Brian Vandre
2014-10-21  7:29 ` [PATCH 0/2] Staging: iio: adc: Checkpatch.pl warning cleanups Alexandre Belloni
2014-10-21 22:56 ` [PATCH v2] " Brian Vandre
2014-10-21 22:56   ` [PATCH v2] Staging: iio: adc: fix line over 80 characters Brian Vandre
2014-10-22  4:21     ` Sudip Mukherjee
2014-10-22 23:47       ` Hartmut Knaack
2014-10-23  2:02         ` Brian Vandre
2014-10-23  3:23         ` Joe Perches
2014-10-23  3:23           ` Joe Perches
2014-10-23  9:46         ` Sudip Mukherjee
2014-10-23 10:50       ` Lars-Peter Clausen
2014-10-23 12:42         ` Brian Vandre
2014-10-23 12:53     ` Brian Vandre
2014-10-23 13:27       ` Alexandre Belloni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.