All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Use macros BIT() and GENMASK()
@ 2017-02-18 20:19 sayli karnik
  2017-02-18 20:20 ` [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: sayli karnik @ 2017-02-18 20:19 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	linux-iio

Use BIT() and GENMASK() macros for left shifts. Also add a blank line after a
function declaration.

sayli karnik (3):
  staging: iio: ad7152: Use BIT() macro for left shifting 1
  staging: iio: ad7152: Use GENMASK() macro for left shifts
  staging: iio: ad7152: Add a blank line after a function

 drivers/staging/iio/cdc/ad7152.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

-- 
2.7.4



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1
  2017-02-18 20:19 [PATCH 0/3] Use macros BIT() and GENMASK() sayli karnik
@ 2017-02-18 20:20 ` sayli karnik
  2017-02-18 20:35   ` Lars-Peter Clausen
  2017-02-18 20:21 ` [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik
  2017-02-18 20:22 ` [PATCH 3/3] staging: iio: ad7152: Add a blank line after a function definition sayli karnik
  2 siblings, 1 reply; 9+ messages in thread
From: sayli karnik @ 2017-02-18 20:20 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	linux-iio

Replace left shifting on 1 with the BIT(x) macro as suggested by
checkpatch.pl.

Done with coccinelle:
@@ int g; @@

-(1 << g)
+BIT(g)

Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
---
 drivers/staging/iio/cdc/ad7152.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index b91b50f..0c97d5a 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -41,30 +41,30 @@
 #define AD7152_REG_CFG2			26
 
 /* Status Register Bit Designations (AD7152_REG_STATUS) */
-#define AD7152_STATUS_RDY1		(1 << 0)
-#define AD7152_STATUS_RDY2		(1 << 1)
-#define AD7152_STATUS_C1C2		(1 << 2)
-#define AD7152_STATUS_PWDN		(1 << 7)
+#define AD7152_STATUS_RDY1		BIT(0)
+#define AD7152_STATUS_RDY2		BIT(1)
+#define AD7152_STATUS_C1C2		BIT(2)
+#define AD7152_STATUS_PWDN		BIT(7)
 
 /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */
-#define AD7152_SETUP_CAPDIFF		(1 << 5)
+#define AD7152_SETUP_CAPDIFF		BIT(5)
 #define AD7152_SETUP_RANGE_2pF		(0 << 6)
-#define AD7152_SETUP_RANGE_0_5pF	(1 << 6)
+#define AD7152_SETUP_RANGE_0_5pF	BIT(6)
 #define AD7152_SETUP_RANGE_1pF		(2 << 6)
 #define AD7152_SETUP_RANGE_4pF		(3 << 6)
 #define AD7152_SETUP_RANGE(x)		((x) << 6)
 
 /* Config Register Bit Designations (AD7152_REG_CFG) */
-#define AD7152_CONF_CH2EN		(1 << 3)
-#define AD7152_CONF_CH1EN		(1 << 4)
+#define AD7152_CONF_CH2EN		BIT(3)
+#define AD7152_CONF_CH1EN		BIT(4)
 #define AD7152_CONF_MODE_IDLE		(0 << 0)
-#define AD7152_CONF_MODE_CONT_CONV	(1 << 0)
+#define AD7152_CONF_MODE_CONT_CONV	BIT(0)
 #define AD7152_CONF_MODE_SINGLE_CONV	(2 << 0)
 #define AD7152_CONF_MODE_OFFS_CAL	(5 << 0)
 #define AD7152_CONF_MODE_GAIN_CAL	(6 << 0)
 
 /* Capdac Register Bit Designations (AD7152_REG_CAPDAC_XXX) */
-#define AD7152_CAPDAC_DACEN		(1 << 7)
+#define AD7152_CAPDAC_DACEN		BIT(7)
 #define AD7152_CAPDAC_DACP(x)		((x) & 0x1F)
 
 /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts
  2017-02-18 20:19 [PATCH 0/3] Use macros BIT() and GENMASK() sayli karnik
  2017-02-18 20:20 ` [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik
@ 2017-02-18 20:21 ` sayli karnik
  2017-02-18 20:35   ` [Outreachy kernel] " Julia Lawall
  2017-02-18 20:40   ` Lars-Peter Clausen
  2017-02-18 20:22 ` [PATCH 3/3] staging: iio: ad7152: Add a blank line after a function definition sayli karnik
  2 siblings, 2 replies; 9+ messages in thread
From: sayli karnik @ 2017-02-18 20:21 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	linux-iio

Use GENMASK() macro for left shifting integers.
Done using coccinelle:
@@ int a,b; @@

-(a << b)
+GENMASK(a, b)

Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
---
 drivers/staging/iio/cdc/ad7152.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index 0c97d5a..988b89b 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -48,27 +48,27 @@
 
 /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */
 #define AD7152_SETUP_CAPDIFF		BIT(5)
-#define AD7152_SETUP_RANGE_2pF		(0 << 6)
+#define AD7152_SETUP_RANGE_2pF		GENMASK(0, 6)
 #define AD7152_SETUP_RANGE_0_5pF	BIT(6)
-#define AD7152_SETUP_RANGE_1pF		(2 << 6)
-#define AD7152_SETUP_RANGE_4pF		(3 << 6)
+#define AD7152_SETUP_RANGE_1pF		GENMASK(2, 6)
+#define AD7152_SETUP_RANGE_4pF		GENMASK(3, 6)
 #define AD7152_SETUP_RANGE(x)		((x) << 6)
 
 /* Config Register Bit Designations (AD7152_REG_CFG) */
 #define AD7152_CONF_CH2EN		BIT(3)
 #define AD7152_CONF_CH1EN		BIT(4)
-#define AD7152_CONF_MODE_IDLE		(0 << 0)
+#define AD7152_CONF_MODE_IDLE		GENMASK(0, 0)
 #define AD7152_CONF_MODE_CONT_CONV	BIT(0)
-#define AD7152_CONF_MODE_SINGLE_CONV	(2 << 0)
-#define AD7152_CONF_MODE_OFFS_CAL	(5 << 0)
-#define AD7152_CONF_MODE_GAIN_CAL	(6 << 0)
+#define AD7152_CONF_MODE_SINGLE_CONV	GENMASK(2, 0)
+#define AD7152_CONF_MODE_OFFS_CAL	GENMASK(5, 0)
+#define AD7152_CONF_MODE_GAIN_CAL	GENMASK(6, 0)
 
 /* Capdac Register Bit Designations (AD7152_REG_CAPDAC_XXX) */
 #define AD7152_CAPDAC_DACEN		BIT(7)
 #define AD7152_CAPDAC_DACP(x)		((x) & 0x1F)
 
 /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
-#define AD7152_CFG2_OSR(x)		(((x) & 0x3) << 4)
+#define AD7152_CFG2_OSR(x)		GENMASK(((x) & 0x3), 4)
 
 enum {
 	AD7152_DATA,
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] staging: iio: ad7152: Add a blank line after a function definition
  2017-02-18 20:19 [PATCH 0/3] Use macros BIT() and GENMASK() sayli karnik
  2017-02-18 20:20 ` [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik
  2017-02-18 20:21 ` [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik
@ 2017-02-18 20:22 ` sayli karnik
  2 siblings, 0 replies; 9+ messages in thread
From: sayli karnik @ 2017-02-18 20:22 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	linux-iio

The patch resolves the checkpatch issue:
CHECK: Please use a blank line after function/struct/union/enum declarations.

Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
---
 drivers/staging/iio/cdc/ad7152.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index 988b89b..2ed6a90 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -244,6 +244,7 @@ static int ad7152_write_raw_samp_freq(struct device *dev, int val)
 
 	return ret;
 }
+
 static int ad7152_write_raw(struct iio_dev *indio_dev,
 			    struct iio_chan_spec const *chan,
 			    int val,
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1
  2017-02-18 20:20 ` [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik
@ 2017-02-18 20:35   ` Lars-Peter Clausen
  0 siblings, 0 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2017-02-18 20:35 UTC (permalink / raw)
  To: sayli karnik, outreachy-kernel
  Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio

On 02/18/2017 09:20 PM, sayli karnik wrote:
> Replace left shifting on 1 with the BIT(x) macro as suggested by
> checkpatch.pl.
> 
> Done with coccinelle:
> @@ int g; @@
> 
> -(1 << g)
> +BIT(g)
> 
> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>

Hi,

Thanks for the patch. When doing these kind of conversions special care
needs to be taken to make sure that they make semantical sense.

The BIT(x) transform for example should only be done for single bit bitfields.

[...]
>  /* Status Register Bit Designations (AD7152_REG_STATUS) */
> -#define AD7152_STATUS_RDY1		(1 << 0)
> -#define AD7152_STATUS_RDY2		(1 << 1)
> -#define AD7152_STATUS_C1C2		(1 << 2)
> -#define AD7152_STATUS_PWDN		(1 << 7)
> +#define AD7152_STATUS_RDY1		BIT(0)
> +#define AD7152_STATUS_RDY2		BIT(1)
> +#define AD7152_STATUS_C1C2		BIT(2)
> +#define AD7152_STATUS_PWDN		BIT(7)

This is correct, these are all single bit bitfields.

>  
>  /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */
> -#define AD7152_SETUP_CAPDIFF		(1 << 5)
> +#define AD7152_SETUP_CAPDIFF		BIT(5)
>  #define AD7152_SETUP_RANGE_2pF		(0 << 6)
> -#define AD7152_SETUP_RANGE_0_5pF	(1 << 6)
> +#define AD7152_SETUP_RANGE_0_5pF	BIT(6)

This on the other hand is a multi-bit field with 1 being one of the possible
values. Hence this should be part of the second patch and use GENMASK instead.

In general when you have a whole bunch of values following the scheme of y
<< x. With 1 << x being one of them use GENMASK() rather than BIT().

>  #define AD7152_SETUP_RANGE_1pF		(2 << 6)
>  #define AD7152_SETUP_RANGE_4pF		(3 << 6)
>  #define AD7152_SETUP_RANGE(x)		((x) << 6)
>  
>  /* Config Register Bit Designations (AD7152_REG_CFG) */
> -#define AD7152_CONF_CH2EN		(1 << 3)
> -#define AD7152_CONF_CH1EN		(1 << 4)
> +#define AD7152_CONF_CH2EN		BIT(3)
> +#define AD7152_CONF_CH1EN		BIT(4)
>  #define AD7152_CONF_MODE_IDLE		(0 << 0)
> -#define AD7152_CONF_MODE_CONT_CONV	(1 << 0)
> +#define AD7152_CONF_MODE_CONT_CONV	BIT(0)

Same here.

>  #define AD7152_CONF_MODE_SINGLE_CONV	(2 << 0)
>  #define AD7152_CONF_MODE_OFFS_CAL	(5 << 0)
>  #define AD7152_CONF_MODE_GAIN_CAL	(6 << 0)
>  
>  /* Capdac Register Bit Designations (AD7152_REG_CAPDAC_XXX) */
> -#define AD7152_CAPDAC_DACEN		(1 << 7)
> +#define AD7152_CAPDAC_DACEN		BIT(7)
>  #define AD7152_CAPDAC_DACP(x)		((x) & 0x1F)
>  
>  /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Outreachy kernel] [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts
  2017-02-18 20:21 ` [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik
@ 2017-02-18 20:35   ` Julia Lawall
  2017-02-18 20:40   ` Lars-Peter Clausen
  1 sibling, 0 replies; 9+ messages in thread
From: Julia Lawall @ 2017-02-18 20:35 UTC (permalink / raw)
  To: sayli karnik
  Cc: outreachy-kernel, Lars-Peter Clausen, Michael Hennerich,
	Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
	Greg Kroah-Hartman, linux-iio



On Sun, 19 Feb 2017, sayli karnik wrote:

> Use GENMASK() macro for left shifting integers.
> Done using coccinelle:
> @@ int a,b; @@
>
> -(a << b)
> +GENMASK(a, b)
>
> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
> ---
>  drivers/staging/iio/cdc/ad7152.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
> index 0c97d5a..988b89b 100644
> --- a/drivers/staging/iio/cdc/ad7152.c
> +++ b/drivers/staging/iio/cdc/ad7152.c
> @@ -48,27 +48,27 @@
>
>  /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */
>  #define AD7152_SETUP_CAPDIFF		BIT(5)
> -#define AD7152_SETUP_RANGE_2pF		(0 << 6)
> +#define AD7152_SETUP_RANGE_2pF		GENMASK(0, 6)
>  #define AD7152_SETUP_RANGE_0_5pF	BIT(6)
> -#define AD7152_SETUP_RANGE_1pF		(2 << 6)
> -#define AD7152_SETUP_RANGE_4pF		(3 << 6)
> +#define AD7152_SETUP_RANGE_1pF		GENMASK(2, 6)
> +#define AD7152_SETUP_RANGE_4pF		GENMASK(3, 6)
>  #define AD7152_SETUP_RANGE(x)		((x) << 6)

I think it would be more understandable to do all of the cases in the same
way.

julia

>
>  /* Config Register Bit Designations (AD7152_REG_CFG) */
>  #define AD7152_CONF_CH2EN		BIT(3)
>  #define AD7152_CONF_CH1EN		BIT(4)
> -#define AD7152_CONF_MODE_IDLE		(0 << 0)
> +#define AD7152_CONF_MODE_IDLE		GENMASK(0, 0)
>  #define AD7152_CONF_MODE_CONT_CONV	BIT(0)
> -#define AD7152_CONF_MODE_SINGLE_CONV	(2 << 0)
> -#define AD7152_CONF_MODE_OFFS_CAL	(5 << 0)
> -#define AD7152_CONF_MODE_GAIN_CAL	(6 << 0)
> +#define AD7152_CONF_MODE_SINGLE_CONV	GENMASK(2, 0)
> +#define AD7152_CONF_MODE_OFFS_CAL	GENMASK(5, 0)
> +#define AD7152_CONF_MODE_GAIN_CAL	GENMASK(6, 0)
>
>  /* Capdac Register Bit Designations (AD7152_REG_CAPDAC_XXX) */
>  #define AD7152_CAPDAC_DACEN		BIT(7)
>  #define AD7152_CAPDAC_DACP(x)		((x) & 0x1F)
>
>  /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
> -#define AD7152_CFG2_OSR(x)		(((x) & 0x3) << 4)
> +#define AD7152_CFG2_OSR(x)		GENMASK(((x) & 0x3), 4)
>
>  enum {
>  	AD7152_DATA,
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/92240a3dca0138880fe47a7df0a5e6df869c8199.1487448326.git.karniksayli1995%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts
  2017-02-18 20:21 ` [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik
  2017-02-18 20:35   ` [Outreachy kernel] " Julia Lawall
@ 2017-02-18 20:40   ` Lars-Peter Clausen
  2017-02-19  6:01     ` [Outreachy kernel] " sayli karnik
  1 sibling, 1 reply; 9+ messages in thread
From: Lars-Peter Clausen @ 2017-02-18 20:40 UTC (permalink / raw)
  To: sayli karnik, outreachy-kernel
  Cc: Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
	Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio

On 02/18/2017 09:21 PM, sayli karnik wrote:
> Use GENMASK() macro for left shifting integers.
> Done using coccinelle:
> @@ int a,b; @@
> 
> -(a << b)
> +GENMASK(a, b)
> 
> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>

Hi,

Thanks for the patch. Looks mostly good, but please rework the patch for
version 2 to address the comments for patch 1.

Some additional comments inline.

[...]
>  /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */
>  #define AD7152_SETUP_CAPDIFF		BIT(5)
> -#define AD7152_SETUP_RANGE_2pF		(0 << 6)
> +#define AD7152_SETUP_RANGE_2pF		GENMASK(0, 6)
>  #define AD7152_SETUP_RANGE_0_5pF	BIT(6)
> -#define AD7152_SETUP_RANGE_1pF		(2 << 6)
> -#define AD7152_SETUP_RANGE_4pF		(3 << 6)
> +#define AD7152_SETUP_RANGE_1pF		GENMASK(2, 6)
> +#define AD7152_SETUP_RANGE_4pF		GENMASK(3, 6)
>  #define AD7152_SETUP_RANGE(x)		((x) << 6)

AD7152_SETUP_RANGE() can also use the GENMASK() macro

[...]
>  /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
> -#define AD7152_CFG2_OSR(x)		(((x) & 0x3) << 4)
> +#define AD7152_CFG2_OSR(x)		GENMASK(((x) & 0x3), 4)

The extra parenthesis around (x) & 0x3 are no needed when using the macro
(the macro will add them).



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts
  2017-02-18 20:40   ` Lars-Peter Clausen
@ 2017-02-19  6:01     ` sayli karnik
  2017-02-19  9:29       ` Lars-Peter Clausen
  0 siblings, 1 reply; 9+ messages in thread
From: sayli karnik @ 2017-02-19  6:01 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: outreachy-kernel, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	linux-iio

On Sun, Feb 19, 2017 at 2:10 AM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> On 02/18/2017 09:21 PM, sayli karnik wrote:
>> Use GENMASK() macro for left shifting integers.
>> Done using coccinelle:
>> @@ int a,b; @@
>>
>> -(a << b)
>> +GENMASK(a, b)
>>
>> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
>
> Hi,
>
> Thanks for the patch. Looks mostly good, but please rework the patch for
> version 2 to address the comments for patch 1.
>
Yes I am sending another version.
> Some additional comments inline.
>
> [...]
>>  /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */
>>  #define AD7152_SETUP_CAPDIFF         BIT(5)
>> -#define AD7152_SETUP_RANGE_2pF               (0 << 6)
>> +#define AD7152_SETUP_RANGE_2pF               GENMASK(0, 6)
>>  #define AD7152_SETUP_RANGE_0_5pF     BIT(6)
>> -#define AD7152_SETUP_RANGE_1pF               (2 << 6)
>> -#define AD7152_SETUP_RANGE_4pF               (3 << 6)
>> +#define AD7152_SETUP_RANGE_1pF               GENMASK(2, 6)
>> +#define AD7152_SETUP_RANGE_4pF               GENMASK(3, 6)
>>  #define AD7152_SETUP_RANGE(x)                ((x) << 6)
>
> AD7152_SETUP_RANGE() can also use the GENMASK() macro
>
> [...]
>>  /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
>> -#define AD7152_CFG2_OSR(x)           (((x) & 0x3) << 4)
>> +#define AD7152_CFG2_OSR(x)           GENMASK(((x) & 0x3), 4)
>
> The extra parenthesis around (x) & 0x3 are no needed when using the macro
> (the macro will add them).
>
The outer parentheses are certainly not needed but for the ones around
x, checkpatch.pl complains otherwise-
CHECK: Macro argument 'x' may be better as '(x)' to avoid precedence issues

Should I still remove them?

thanks,
sayli
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/0b5ef4d0-14b5-1196-8bc4-deda9d8f75ce%40metafoo.de.
> For more options, visit https://groups.google.com/d/optout.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts
  2017-02-19  6:01     ` [Outreachy kernel] " sayli karnik
@ 2017-02-19  9:29       ` Lars-Peter Clausen
  0 siblings, 0 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2017-02-19  9:29 UTC (permalink / raw)
  To: sayli karnik
  Cc: outreachy-kernel, Michael Hennerich, Jonathan Cameron,
	Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman,
	linux-iio

On 02/19/2017 07:01 AM, sayli karnik wrote:
[...]
>> [...]
>>>  /* CFG2 Register Bit Designations (AD7152_REG_CFG2) */
>>> -#define AD7152_CFG2_OSR(x)           (((x) & 0x3) << 4)
>>> +#define AD7152_CFG2_OSR(x)           GENMASK(((x) & 0x3), 4)
>>
>> The extra parenthesis around (x) & 0x3 are no needed when using the macro
>> (the macro will add them).
>>
> The outer parentheses are certainly not needed but for the ones around
> x, checkpatch.pl complains otherwise-
> CHECK: Macro argument 'x' may be better as '(x)' to avoid precedence issues

The one around x are needed and should stay.



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-02-19  9:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-18 20:19 [PATCH 0/3] Use macros BIT() and GENMASK() sayli karnik
2017-02-18 20:20 ` [PATCH 1/3] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik
2017-02-18 20:35   ` Lars-Peter Clausen
2017-02-18 20:21 ` [PATCH 2/3] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik
2017-02-18 20:35   ` [Outreachy kernel] " Julia Lawall
2017-02-18 20:40   ` Lars-Peter Clausen
2017-02-19  6:01     ` [Outreachy kernel] " sayli karnik
2017-02-19  9:29       ` Lars-Peter Clausen
2017-02-18 20:22 ` [PATCH 3/3] staging: iio: ad7152: Add a blank line after a function definition sayli karnik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.