* [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 related [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
* [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 related [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).