linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2]Staging:iio:adc:add space around '-'
@ 2016-01-15 19:42 Bhumika Goyal
  2016-01-15 19:42 ` [PATCH 1/2] staging:iio:adc:added " Bhumika Goyal
  2016-01-15 19:42 ` [PATCH 2/2] staging:iio:adc:add " Bhumika Goyal
  0 siblings, 2 replies; 11+ messages in thread
From: Bhumika Goyal @ 2016-01-15 19:42 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, gregkh, linux-kernel, linux-iio, pmeerw,
	devel, Bhumika Goyal

These patches adds space around '-' operator.Found using checkpatch.pl.


Bhumika Goyal (2):
  staging:iio:adc:added space around '-'
  staging:iio:adc:add space around '-'

 drivers/staging/iio/adc/ad7192.c  | 2 +-
 drivers/staging/iio/adc/ad7280a.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
1.9.1

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

* [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-15 19:42 [PATCH 0/2]Staging:iio:adc:add space around '-' Bhumika Goyal
@ 2016-01-15 19:42 ` Bhumika Goyal
  2016-01-15 20:15   ` Lars-Peter Clausen
  2016-01-15 19:42 ` [PATCH 2/2] staging:iio:adc:add " Bhumika Goyal
  1 sibling, 1 reply; 11+ messages in thread
From: Bhumika Goyal @ 2016-01-15 19:42 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, gregkh, linux-kernel, linux-iio, pmeerw,
	devel, Bhumika Goyal

This patch adds apace around '-' operator.Found using checkpatch.pl

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/staging/iio/adc/ad7280a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
index f45ebed..0c73bce 100644
--- a/drivers/staging/iio/adc/ad7280a.c
+++ b/drivers/staging/iio/adc/ad7280a.c
@@ -744,14 +744,14 @@ out:
 }
 
 static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
-		in_voltage-voltage_thresh_low_value,
+		in_voltage - voltage_thresh_low_value,
 		S_IRUGO | S_IWUSR,
 		ad7280_read_channel_config,
 		ad7280_write_channel_config,
 		AD7280A_CELL_UNDERVOLTAGE);
 
 static IIO_DEVICE_ATTR_NAMED(in_thresh_high_value,
-		in_voltage-voltage_thresh_high_value,
+		in_voltage - voltage_thresh_high_value,
 		S_IRUGO | S_IWUSR,
 		ad7280_read_channel_config,
 		ad7280_write_channel_config,
-- 
1.9.1

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

* [PATCH 2/2] staging:iio:adc:add space around '-'
  2016-01-15 19:42 [PATCH 0/2]Staging:iio:adc:add space around '-' Bhumika Goyal
  2016-01-15 19:42 ` [PATCH 1/2] staging:iio:adc:added " Bhumika Goyal
@ 2016-01-15 19:42 ` Bhumika Goyal
  1 sibling, 0 replies; 11+ messages in thread
From: Bhumika Goyal @ 2016-01-15 19:42 UTC (permalink / raw)
  To: jic23
  Cc: lars, Michael.Hennerich, gregkh, linux-kernel, linux-iio, pmeerw,
	devel, Bhumika Goyal

This patch adds space around '-' operator.Found using checkpatch.pl

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 9221103..54db0f0 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -376,7 +376,7 @@ ad7192_show_scale_available(struct device *dev,
 }
 
 static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
-			     in_voltage-voltage_scale_available,
+			     in_voltage - voltage_scale_available,
 			     S_IRUGO, ad7192_show_scale_available, NULL, 0);
 
 static IIO_DEVICE_ATTR(in_voltage_scale_available, S_IRUGO,
-- 
1.9.1

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-15 19:42 ` [PATCH 1/2] staging:iio:adc:added " Bhumika Goyal
@ 2016-01-15 20:15   ` Lars-Peter Clausen
  2016-01-20 14:21     ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2016-01-15 20:15 UTC (permalink / raw)
  To: Bhumika Goyal, jic23
  Cc: Michael.Hennerich, gregkh, linux-kernel, linux-iio, pmeerw, devel

On 01/15/2016 08:42 PM, Bhumika Goyal wrote:
> This patch adds apace around '-' operator.Found using checkpatch.pl
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7280a.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> index f45ebed..0c73bce 100644
> --- a/drivers/staging/iio/adc/ad7280a.c
> +++ b/drivers/staging/iio/adc/ad7280a.c
> @@ -744,14 +744,14 @@ out:
>  }
>  
>  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> -		in_voltage-voltage_thresh_low_value,
> +		in_voltage - voltage_thresh_low_value,

Hi,

Thanks for patch. But when sending cleanup patches like this please make
sure that you a) understand what the code does and how your change affects
it and b) as a bare minimum of testing perform a compile test, if possible
also do functional testing.

The patch as it is, is neither semantically nor syntactically correct. As an
exercise please make sure you understand why.

Same for the second patch.

- Lars

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-15 20:15   ` Lars-Peter Clausen
@ 2016-01-20 14:21     ` Dan Carpenter
  2016-01-24 16:36       ` Jonathan Cameron
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2016-01-20 14:21 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Bhumika Goyal, jic23, devel, Michael.Hennerich, linux-iio,
	gregkh, linux-kernel, pmeerw

On Fri, Jan 15, 2016 at 09:15:52PM +0100, Lars-Peter Clausen wrote:
> On 01/15/2016 08:42 PM, Bhumika Goyal wrote:
> > This patch adds apace around '-' operator.Found using checkpatch.pl
> > 
> > Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> > ---
> >  drivers/staging/iio/adc/ad7280a.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
> > index f45ebed..0c73bce 100644
> > --- a/drivers/staging/iio/adc/ad7280a.c
> > +++ b/drivers/staging/iio/adc/ad7280a.c
> > @@ -744,14 +744,14 @@ out:
> >  }
> >  
> >  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
> > -		in_voltage-voltage_thresh_low_value,
> > +		in_voltage - voltage_thresh_low_value,
> 
> Hi,
> 
> Thanks for patch. But when sending cleanup patches like this please make
> sure that you a) understand what the code does and how your change affects
> it and b) as a bare minimum of testing perform a compile test, if possible
> also do functional testing.
> 
> The patch as it is, is neither semantically nor syntactically correct. As an
> exercise please make sure you understand why.

Ugh!

It took me a long time to figure out the bug in this patch...  Why does
that filename have a mix of dashes and underscores?  Too late to fix it
now...  :/

regards,
dan carpenter

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-20 14:21     ` Dan Carpenter
@ 2016-01-24 16:36       ` Jonathan Cameron
  2016-01-24 17:14         ` Lars-Peter Clausen
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2016-01-24 16:36 UTC (permalink / raw)
  To: Dan Carpenter, Lars-Peter Clausen
  Cc: Bhumika Goyal, devel, Michael.Hennerich, linux-iio, gregkh,
	linux-kernel, pmeerw

On 20/01/16 14:21, Dan Carpenter wrote:
> On Fri, Jan 15, 2016 at 09:15:52PM +0100, Lars-Peter Clausen wrote:
>> On 01/15/2016 08:42 PM, Bhumika Goyal wrote:
>>> This patch adds apace around '-' operator.Found using checkpatch.pl
>>>
>>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>>> ---
>>>  drivers/staging/iio/adc/ad7280a.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
>>> index f45ebed..0c73bce 100644
>>> --- a/drivers/staging/iio/adc/ad7280a.c
>>> +++ b/drivers/staging/iio/adc/ad7280a.c
>>> @@ -744,14 +744,14 @@ out:
>>>  }
>>>  
>>>  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
>>> -		in_voltage-voltage_thresh_low_value,
>>> +		in_voltage - voltage_thresh_low_value,
>>
>> Hi,
>>
>> Thanks for patch. But when sending cleanup patches like this please make
>> sure that you a) understand what the code does and how your change affects
>> it and b) as a bare minimum of testing perform a compile test, if possible
>> also do functional testing.
>>
>> The patch as it is, is neither semantically nor syntactically correct. As an
>> exercise please make sure you understand why.
> 
> Ugh!
> 
> It took me a long time to figure out the bug in this patch...  Why does
> that filename have a mix of dashes and underscores?  Too late to fix it
> now...  :/
> 
Very deliberately.  The - is indicating it is a differential channel!
Literally A minus B.

It's an awfully compact representation for maths ;)
This is obscured partly in this case as it's specifying an attribute
shared by a set of differential channels so it's the generalization
of
in_voltage0-voltage1_thresh_low_value
which does begin to slightly stretch the argument that it is nice and
clear ;(

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

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-24 16:36       ` Jonathan Cameron
@ 2016-01-24 17:14         ` Lars-Peter Clausen
  2016-01-30 14:12           ` Jonathan Cameron
  0 siblings, 1 reply; 11+ messages in thread
From: Lars-Peter Clausen @ 2016-01-24 17:14 UTC (permalink / raw)
  To: Jonathan Cameron, Dan Carpenter
  Cc: Bhumika Goyal, devel, Michael.Hennerich, linux-iio, gregkh,
	linux-kernel, pmeerw

On 01/24/2016 05:36 PM, Jonathan Cameron wrote:
> On 20/01/16 14:21, Dan Carpenter wrote:
>> On Fri, Jan 15, 2016 at 09:15:52PM +0100, Lars-Peter Clausen wrote:
>>> On 01/15/2016 08:42 PM, Bhumika Goyal wrote:
>>>> This patch adds apace around '-' operator.Found using checkpatch.pl
>>>>
>>>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>>>> ---
>>>>  drivers/staging/iio/adc/ad7280a.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
>>>> index f45ebed..0c73bce 100644
>>>> --- a/drivers/staging/iio/adc/ad7280a.c
>>>> +++ b/drivers/staging/iio/adc/ad7280a.c
>>>> @@ -744,14 +744,14 @@ out:
>>>>  }
>>>>  
>>>>  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
>>>> -		in_voltage-voltage_thresh_low_value,
>>>> +		in_voltage - voltage_thresh_low_value,
>>>
>>> Hi,
>>>
>>> Thanks for patch. But when sending cleanup patches like this please make
>>> sure that you a) understand what the code does and how your change affects
>>> it and b) as a bare minimum of testing perform a compile test, if possible
>>> also do functional testing.
>>>
>>> The patch as it is, is neither semantically nor syntactically correct. As an
>>> exercise please make sure you understand why.
>>
>> Ugh!
>>
>> It took me a long time to figure out the bug in this patch...  Why does
>> that filename have a mix of dashes and underscores?  Too late to fix it
>> now...  :/
>>
> Very deliberately.  The - is indicating it is a differential channel!
> Literally A minus B.
> 
> It's an awfully compact representation for maths ;)
> This is obscured partly in this case as it's specifying an attribute
> shared by a set of differential channels so it's the generalization
> of
> in_voltage0-voltage1_thresh_low_value
> which does begin to slightly stretch the argument that it is nice and
> clear ;(

One thing we should maybe take a look at is making it explicit that this is
a string so it does not get picked up by checkpatch.

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-24 17:14         ` Lars-Peter Clausen
@ 2016-01-30 14:12           ` Jonathan Cameron
  2016-01-30 15:12             ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2016-01-30 14:12 UTC (permalink / raw)
  To: Lars-Peter Clausen, Dan Carpenter
  Cc: Bhumika Goyal, devel, Michael.Hennerich, linux-iio, gregkh,
	linux-kernel, pmeerw

On 24/01/16 17:14, Lars-Peter Clausen wrote:
> On 01/24/2016 05:36 PM, Jonathan Cameron wrote:
>> On 20/01/16 14:21, Dan Carpenter wrote:
>>> On Fri, Jan 15, 2016 at 09:15:52PM +0100, Lars-Peter Clausen wrote:
>>>> On 01/15/2016 08:42 PM, Bhumika Goyal wrote:
>>>>> This patch adds apace around '-' operator.Found using checkpatch.pl
>>>>>
>>>>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>>>>> ---
>>>>>  drivers/staging/iio/adc/ad7280a.c | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/iio/adc/ad7280a.c b/drivers/staging/iio/adc/ad7280a.c
>>>>> index f45ebed..0c73bce 100644
>>>>> --- a/drivers/staging/iio/adc/ad7280a.c
>>>>> +++ b/drivers/staging/iio/adc/ad7280a.c
>>>>> @@ -744,14 +744,14 @@ out:
>>>>>  }
>>>>>  
>>>>>  static IIO_DEVICE_ATTR_NAMED(in_thresh_low_value,
>>>>> -		in_voltage-voltage_thresh_low_value,
>>>>> +		in_voltage - voltage_thresh_low_value,
>>>>
>>>> Hi,
>>>>
>>>> Thanks for patch. But when sending cleanup patches like this please make
>>>> sure that you a) understand what the code does and how your change affects
>>>> it and b) as a bare minimum of testing perform a compile test, if possible
>>>> also do functional testing.
>>>>
>>>> The patch as it is, is neither semantically nor syntactically correct. As an
>>>> exercise please make sure you understand why.
>>>
>>> Ugh!
>>>
>>> It took me a long time to figure out the bug in this patch...  Why does
>>> that filename have a mix of dashes and underscores?  Too late to fix it
>>> now...  :/
>>>
>> Very deliberately.  The - is indicating it is a differential channel!
>> Literally A minus B.
>>
>> It's an awfully compact representation for maths ;)
>> This is obscured partly in this case as it's specifying an attribute
>> shared by a set of differential channels so it's the generalization
>> of
>> in_voltage0-voltage1_thresh_low_value
>> which does begin to slightly stretch the argument that it is nice and
>> clear ;(
> 
> One thing we should maybe take a look at is making it explicit that this is
> a string so it does not get picked up by checkpatch.
Make sense. Patches welcome :)
> 
> --
> 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] 11+ messages in thread

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-30 14:12           ` Jonathan Cameron
@ 2016-01-30 15:12             ` Dan Carpenter
  2016-01-30 15:15               ` Lars-Peter Clausen
  2016-01-30 17:02               ` Joe Perches
  0 siblings, 2 replies; 11+ messages in thread
From: Dan Carpenter @ 2016-01-30 15:12 UTC (permalink / raw)
  To: Jonathan Cameron, Joe Perches
  Cc: Lars-Peter Clausen, devel, Michael.Hennerich, linux-iio, gregkh,
	linux-kernel, pmeerw, Bhumika Goyal

We could make checkpatch.pl not complain if the line says checkpatch: on
it.  It would look like this.

-               in_voltage-voltage_thresh_low_value,
+               in_voltage-voltage_thresh_low_value, /* checkpatch: not math */

I suppose I could have made the explanation longer since the it won't
complain about the 80 character limit...  What do you guys think?

regards,
dan carpenter

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-30 15:12             ` Dan Carpenter
@ 2016-01-30 15:15               ` Lars-Peter Clausen
  2016-01-30 17:02               ` Joe Perches
  1 sibling, 0 replies; 11+ messages in thread
From: Lars-Peter Clausen @ 2016-01-30 15:15 UTC (permalink / raw)
  To: Dan Carpenter, Jonathan Cameron, Joe Perches
  Cc: devel, Michael.Hennerich, linux-iio, gregkh, linux-kernel,
	pmeerw, Bhumika Goyal

On 01/30/2016 04:12 PM, Dan Carpenter wrote:
> We could make checkpatch.pl not complain if the line says checkpatch: on
> it.  It would look like this.
> 
> -               in_voltage-voltage_thresh_low_value,
> +               in_voltage-voltage_thresh_low_value, /* checkpatch: not math */
> 
> I suppose I could have made the explanation longer since the it won't
> complain about the 80 character limit...  What do you guys think?

We could add it as a temporary way to silence the checker. But it feels a
bit ugly, there is really no reason why this shouldn't be a string, other
than that the current device attr macros don't support that.

- Lars

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

* Re: [PATCH 1/2] staging:iio:adc:added space around '-'
  2016-01-30 15:12             ` Dan Carpenter
  2016-01-30 15:15               ` Lars-Peter Clausen
@ 2016-01-30 17:02               ` Joe Perches
  1 sibling, 0 replies; 11+ messages in thread
From: Joe Perches @ 2016-01-30 17:02 UTC (permalink / raw)
  To: Dan Carpenter, Jonathan Cameron
  Cc: Lars-Peter Clausen, devel, Michael.Hennerich, linux-iio, gregkh,
	linux-kernel, pmeerw, Bhumika Goyal

On Sat, 2016-01-30 at 18:12 +0300, Dan Carpenter wrote:
> We could make checkpatch.pl not complain if the line says checkpatch:
> on
> it.  It would look like this.
> 
> -               in_voltage-voltage_thresh_low_value,
> +               in_voltage-voltage_thresh_low_value, /* checkpatch:
> not math */
> 
> I suppose I could have made the explanation longer since the it won't
> complain about the 80 character limit...  What do yo/u guys think?

Maybe use a more generic thing like the checkpatch type

               in_voltage-voltage_thresh_low_value, /* checkpatch-SPACING */

Even so, it might uglify checkpatch code a lot to
check something like this per-line or per-block.

And that likely would have to be per line in the
code as checkpatch couldn't see when a patch block
addition occurs outside the scope of a comment.

I suppose inside checkpatch the "sub report {"
function could be extended to look at the specific
$rawline being tested for any "checkpatch" comment
and if so, test if it's the specific $type.

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

end of thread, other threads:[~2016-01-30 17:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-15 19:42 [PATCH 0/2]Staging:iio:adc:add space around '-' Bhumika Goyal
2016-01-15 19:42 ` [PATCH 1/2] staging:iio:adc:added " Bhumika Goyal
2016-01-15 20:15   ` Lars-Peter Clausen
2016-01-20 14:21     ` Dan Carpenter
2016-01-24 16:36       ` Jonathan Cameron
2016-01-24 17:14         ` Lars-Peter Clausen
2016-01-30 14:12           ` Jonathan Cameron
2016-01-30 15:12             ` Dan Carpenter
2016-01-30 15:15               ` Lars-Peter Clausen
2016-01-30 17:02               ` Joe Perches
2016-01-15 19:42 ` [PATCH 2/2] staging:iio:adc:add " Bhumika Goyal

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