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 ;( Jonathan > regards, > dan carpenter > > -- > 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
