Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1310406 > unrolled thread
| Started by | Bhumika Goyal <bhumirks@gmail.com> |
|---|---|
| First post | 2016-01-15 20:50 +0100 |
| Last post | 2016-01-24 19:00 +0100 |
| Articles | 5 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/2] staging:iio:adc:added space around '-' Bhumika Goyal <bhumirks@gmail.com> - 2016-01-15 20:50 +0100
Re: [PATCH 1/2] staging:iio:adc:added space around '-' Lars-Peter Clausen <lars@metafoo.de> - 2016-01-15 21:20 +0100
Re: [PATCH 1/2] staging:iio:adc:added space around '-' Dan Carpenter <dan.carpenter@oracle.com> - 2016-01-20 15:30 +0100
Re: [PATCH 1/2] staging:iio:adc:added space around '-' Jonathan Cameron <jic23@kernel.org> - 2016-01-24 17:40 +0100
Re: [PATCH 1/2] staging:iio:adc:added space around '-' Lars-Peter Clausen <lars@metafoo.de> - 2016-01-24 19:00 +0100
| From | Bhumika Goyal <bhumirks@gmail.com> |
|---|---|
| Date | 2016-01-15 20:50 +0100 |
| Subject | [PATCH 1/2] staging:iio:adc:added space around '-' |
| Message-ID | <qRixH-2cl-1@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Lars-Peter Clausen <lars@metafoo.de> |
|---|---|
| Date | 2016-01-15 21:20 +0100 |
| Message-ID | <qRj0K-2C0-11@gated-at.bofh.it> |
| In reply to | #1310406 |
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
[toc] | [prev] | [next] | [standalone]
| From | Dan Carpenter <dan.carpenter@oracle.com> |
|---|---|
| Date | 2016-01-20 15:30 +0100 |
| Message-ID | <qT1VM-7Ky-9@gated-at.bofh.it> |
| In reply to | #1310442 |
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
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-01-24 17:40 +0100 |
| Message-ID | <qUvRL-7sW-7@gated-at.bofh.it> |
| In reply to | #1313245 |
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 >
[toc] | [prev] | [next] | [standalone]
| From | Lars-Peter Clausen <lars@metafoo.de> |
|---|---|
| Date | 2016-01-24 19:00 +0100 |
| Message-ID | <qUx7c-8gy-7@gated-at.bofh.it> |
| In reply to | #1315925 |
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.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web