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@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, >>>> >>>> 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
