Re: [PATCH 1/2] staging:iio:adc:added space around '-'
From: Lars-Peter Clausen
Date: Sun Jan 24 2016 - 12:52:27 EST
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@xxxxxxxxx>
>>>> 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,
>>> 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.
>> 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
> 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.