Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
Looking at the driver the define you are fixing isn't even used. So it could be removed. While you are at it you may want to remove the peice of completey redundant code. . . if (ret) goto error_ret; error_ret: . . Wow. On Sat, Jun 28, 2014 at 05:10:46AM +0200, Guillaume Morin wrote: > On 27 Jun 22:37, Greg Kroah-Hartman wrote: > > Put that below the --- line. > > Will do. > > > > > And what checkpatch error did you fix? And are you sure it needs to be > > > > fixed? > > > > > > That's what I changed: > > > > > > $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c > > > ERROR: Macros with complex values should be enclosed in parenthesis > > > > Then why didn't you say that :) > > Well it was not totally clear to me if that was obvious or not. Anyway, > I'll mention it in the future. > > > > > > I assumed that if it was reported as an error, it needed to be fixed... > > > > Use your judgement, checkpatch is a tool, it isn't always correct. > > Right, I guess it's borderline. Should I resend the patch or just drop > it? > > Guillaume. > > -- > Guillaume Morin > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On Sat, Jun 28, 2014 at 05:10:46AM +0200, Guillaume Morin wrote: > On 27 Jun 22:37, Greg Kroah-Hartman wrote: > > Put that below the --- line. > > Will do. > > > > > And what checkpatch error did you fix? And are you sure it needs to be > > > > fixed? > > > > > > That's what I changed: > > > > > > $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c > > > ERROR: Macros with complex values should be enclosed in parenthesis > > > > Then why didn't you say that :) > > Well it was not totally clear to me if that was obvious or not. Anyway, > I'll mention it in the future. > > > > > > I assumed that if it was reported as an error, it needed to be fixed... > > > > Use your judgement, checkpatch is a tool, it isn't always correct. > > Right, I guess it's borderline. Should I resend the patch or just drop > it? These days we have GENMASK. http://lxr.free-electrons.com/source/include/linux/bitops.h#L21 Maybe the macro can be used directly instead of the value_mask. > > Guillaume. > > -- > Guillaume Morin > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On Sat, Jun 28, 2014 at 05:10:46AM +0200, Guillaume Morin wrote: On 27 Jun 22:37, Greg Kroah-Hartman wrote: Put that below the --- line. Will do. And what checkpatch error did you fix? And are you sure it needs to be fixed? That's what I changed: $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c ERROR: Macros with complex values should be enclosed in parenthesis Then why didn't you say that :) Well it was not totally clear to me if that was obvious or not. Anyway, I'll mention it in the future. I assumed that if it was reported as an error, it needed to be fixed... Use your judgement, checkpatch is a tool, it isn't always correct. Right, I guess it's borderline. Should I resend the patch or just drop it? These days we have GENMASK. http://lxr.free-electrons.com/source/include/linux/bitops.h#L21 Maybe the macro can be used directly instead of the value_mask. Guillaume. -- Guillaume Morin guilla...@morinfr.org -- To unsubscribe from this list: send the line unsubscribe linux-iio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
Looking at the driver the define you are fixing isn't even used. So it could be removed. While you are at it you may want to remove the peice of completey redundant code. . . if (ret) goto error_ret; error_ret: . . Wow. On Sat, Jun 28, 2014 at 05:10:46AM +0200, Guillaume Morin wrote: On 27 Jun 22:37, Greg Kroah-Hartman wrote: Put that below the --- line. Will do. And what checkpatch error did you fix? And are you sure it needs to be fixed? That's what I changed: $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c ERROR: Macros with complex values should be enclosed in parenthesis Then why didn't you say that :) Well it was not totally clear to me if that was obvious or not. Anyway, I'll mention it in the future. I assumed that if it was reported as an error, it needed to be fixed... Use your judgement, checkpatch is a tool, it isn't always correct. Right, I guess it's borderline. Should I resend the patch or just drop it? Guillaume. -- Guillaume Morin guilla...@morinfr.org -- To unsubscribe from this list: send the line unsubscribe linux-iio in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On 27 Jun 22:37, Greg Kroah-Hartman wrote: > Put that below the --- line. Will do. > > > And what checkpatch error did you fix? And are you sure it needs to be > > > fixed? > > > > That's what I changed: > > > > $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c > > ERROR: Macros with complex values should be enclosed in parenthesis > > Then why didn't you say that :) Well it was not totally clear to me if that was obvious or not. Anyway, I'll mention it in the future. > > > I assumed that if it was reported as an error, it needed to be fixed... > > Use your judgement, checkpatch is a tool, it isn't always correct. Right, I guess it's borderline. Should I resend the patch or just drop it? Guillaume. -- Guillaume Morin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On Sat, Jun 28, 2014 at 04:30:09AM +0200, Guillaume Morin wrote: > On 27 Jun 19:09, Greg Kroah-Hartman wrote: > > > v2: add missing Signed-off-by > > > > That doesn't go here. > > I guess I am struggling to get git send-email do what I want Put that below the --- line. > > And what checkpatch error did you fix? And are you sure it needs to be > > fixed? > > That's what I changed: > > $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c > ERROR: Macros with complex values should be enclosed in parenthesis Then why didn't you say that :) > I assumed that if it was reported as an error, it needed to be fixed... Use your judgement, checkpatch is a tool, it isn't always correct. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On 27 Jun 19:09, Greg Kroah-Hartman wrote: > > v2: add missing Signed-off-by > > That doesn't go here. I guess I am struggling to get git send-email do what I want > And what checkpatch error did you fix? And are you sure it needs to be > fixed? That's what I changed: $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c ERROR: Macros with complex values should be enclosed in parenthesis #24: FILE: drivers/staging/iio/frequency/ad9850.c:24: +#define value_mask (u16)0xf000 I assumed that if it was reported as an error, it needed to be fixed... -- Guillaume Morin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On Sat, Jun 28, 2014 at 03:46:56AM +0200, Guillaume Morin wrote: > v2: add missing Signed-off-by That doesn't go here. And what checkpatch error did you fix? And are you sure it needs to be fixed? greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
v2: add missing Signed-off-by Signed-off-by: Guillaume Morin diff --git a/drivers/staging/iio/frequency/ad9850.c b/drivers/staging/iio/frequency/ad9850.c index af877ff..6183670 100644 --- a/drivers/staging/iio/frequency/ad9850.c +++ b/drivers/staging/iio/frequency/ad9850.c @@ -21,7 +21,7 @@ #define DRV_NAME "ad9850" -#define value_mask (u16)0xf000 +#define value_mask ((u16)0xf000) #define addr_shift 12 /* Register format: 4 bits addr + 12 bits value */ -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
v2: add missing Signed-off-by Signed-off-by: Guillaume Morin guilla...@morinfr.org diff --git a/drivers/staging/iio/frequency/ad9850.c b/drivers/staging/iio/frequency/ad9850.c index af877ff..6183670 100644 --- a/drivers/staging/iio/frequency/ad9850.c +++ b/drivers/staging/iio/frequency/ad9850.c @@ -21,7 +21,7 @@ #define DRV_NAME ad9850 -#define value_mask (u16)0xf000 +#define value_mask ((u16)0xf000) #define addr_shift 12 /* Register format: 4 bits addr + 12 bits value */ -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On Sat, Jun 28, 2014 at 03:46:56AM +0200, Guillaume Morin wrote: v2: add missing Signed-off-by That doesn't go here. And what checkpatch error did you fix? And are you sure it needs to be fixed? greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On 27 Jun 19:09, Greg Kroah-Hartman wrote: v2: add missing Signed-off-by That doesn't go here. I guess I am struggling to get git send-email do what I want And what checkpatch error did you fix? And are you sure it needs to be fixed? That's what I changed: $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c ERROR: Macros with complex values should be enclosed in parenthesis #24: FILE: drivers/staging/iio/frequency/ad9850.c:24: +#define value_mask (u16)0xf000 I assumed that if it was reported as an error, it needed to be fixed... -- Guillaume Morin guilla...@morinfr.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On Sat, Jun 28, 2014 at 04:30:09AM +0200, Guillaume Morin wrote: On 27 Jun 19:09, Greg Kroah-Hartman wrote: v2: add missing Signed-off-by That doesn't go here. I guess I am struggling to get git send-email do what I want Put that below the --- line. And what checkpatch error did you fix? And are you sure it needs to be fixed? That's what I changed: $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c ERROR: Macros with complex values should be enclosed in parenthesis Then why didn't you say that :) I assumed that if it was reported as an error, it needed to be fixed... Use your judgement, checkpatch is a tool, it isn't always correct. greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/1] staging: iio: ad9850.c: fix checkpatch.pl error
On 27 Jun 22:37, Greg Kroah-Hartman wrote: Put that below the --- line. Will do. And what checkpatch error did you fix? And are you sure it needs to be fixed? That's what I changed: $ scripts/checkpatch.pl -f drivers/staging/iio/frequency/ad9850.c ERROR: Macros with complex values should be enclosed in parenthesis Then why didn't you say that :) Well it was not totally clear to me if that was obvious or not. Anyway, I'll mention it in the future. I assumed that if it was reported as an error, it needed to be fixed... Use your judgement, checkpatch is a tool, it isn't always correct. Right, I guess it's borderline. Should I resend the patch or just drop it? Guillaume. -- Guillaume Morin guilla...@morinfr.org -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
