* [PATCH v2 0/2] Use BIT() and GENMASK() macros @ 2017-02-19 19:57 sayli karnik 2017-02-19 19:58 ` [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik 2017-02-19 20:00 ` [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik 0 siblings, 2 replies; 11+ messages in thread From: sayli karnik @ 2017-02-19 19:57 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. sayli karnik (2): staging: iio: ad7152: Use BIT() macro for left shifting 1 staging: iio: ad7152: Use GENMASK() macro for left shifts drivers/staging/iio/cdc/ad7152.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 2017-02-19 19:57 [PATCH v2 0/2] Use BIT() and GENMASK() macros sayli karnik @ 2017-02-19 19:58 ` sayli karnik 2017-02-19 20:02 ` [Outreachy kernel] " Julia Lawall 2017-02-19 20:00 ` [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik 1 sibling, 1 reply; 11+ messages in thread From: sayli karnik @ 2017-02-19 19:58 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. Signed-off-by: sayli karnik <karniksayli1995@gmail.com> --- v2: Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. Removed extra parentheses around argument to macro drivers/staging/iio/cdc/ad7152.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c index b91b50f..e8609b8 100644 --- a/drivers/staging/iio/cdc/ad7152.c +++ b/drivers/staging/iio/cdc/ad7152.c @@ -41,10 +41,10 @@ #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) -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 2017-02-19 19:58 ` [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik @ 2017-02-19 20:02 ` Julia Lawall 2017-02-25 16:47 ` Jonathan Cameron 0 siblings, 1 reply; 11+ messages in thread From: Julia Lawall @ 2017-02-19 20:02 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 Mon, 20 Feb 2017, sayli karnik wrote: > Replace left shifting on 1 with the BIT(x) macro as suggested by > checkpatch.pl. > > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> Acked-by: Julia Lawall <julia.lawall@lip6.fr> > --- > v2: > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. > Removed extra parentheses around argument to macro > > drivers/staging/iio/cdc/ad7152.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c > index b91b50f..e8609b8 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -41,10 +41,10 @@ > #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) > -- > 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/06283c7cbccd40a7214bf4f047c3829e53b6d448.1487483384.git.karniksayli1995%40gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 2017-02-19 20:02 ` [Outreachy kernel] " Julia Lawall @ 2017-02-25 16:47 ` Jonathan Cameron 0 siblings, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2017-02-25 16:47 UTC (permalink / raw) To: Julia Lawall, sayli karnik Cc: outreachy-kernel, Lars-Peter Clausen, Michael Hennerich, Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio On 19/02/17 20:02, Julia Lawall wrote: > > > On Mon, 20 Feb 2017, sayli karnik wrote: > >> Replace left shifting on 1 with the BIT(x) macro as suggested by >> checkpatch.pl. >> >> Signed-off-by: sayli karnik <karniksayli1995@gmail.com> > > Acked-by: Julia Lawall <julia.lawall@lip6.fr> Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. For obvious reasons I'm going to leave patch 2 alone! Thanks, Jonathan > >> --- >> v2: >> Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. >> Removed extra parentheses around argument to macro >> >> drivers/staging/iio/cdc/ad7152.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c >> index b91b50f..e8609b8 100644 >> --- a/drivers/staging/iio/cdc/ad7152.c >> +++ b/drivers/staging/iio/cdc/ad7152.c >> @@ -41,10 +41,10 @@ >> #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) >> -- >> 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/06283c7cbccd40a7214bf4f047c3829e53b6d448.1487483384.git.karniksayli1995%40gmail.com. >> For more options, visit https://groups.google.com/d/optout. >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-19 19:57 [PATCH v2 0/2] Use BIT() and GENMASK() macros sayli karnik 2017-02-19 19:58 ` [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik @ 2017-02-19 20:00 ` sayli karnik 2017-02-19 20:04 ` [Outreachy kernel] " Julia Lawall ` (2 more replies) 1 sibling, 3 replies; 11+ messages in thread From: sayli karnik @ 2017-02-19 20:00 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. Signed-off-by: sayli karnik <karniksayli1995@gmail.com> --- v2: Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. Removed extra parentheses around argument to macro drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c index e8609b8..ab94fad 100644 --- a/drivers/staging/iio/cdc/ad7152.c +++ b/drivers/staging/iio/cdc/ad7152.c @@ -47,28 +47,28 @@ #define AD7152_STATUS_PWDN BIT(7) /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ -#define AD7152_SETUP_CAPDIFF (1 << 5) -#define AD7152_SETUP_RANGE_2pF (0 << 6) -#define AD7152_SETUP_RANGE_0_5pF (1 << 6) -#define AD7152_SETUP_RANGE_1pF (2 << 6) -#define AD7152_SETUP_RANGE_4pF (3 << 6) -#define AD7152_SETUP_RANGE(x) ((x) << 6) +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) +#define AD7152_SETUP_RANGE_2pF GENMASK(0, 6) +#define AD7152_SETUP_RANGE_0_5pF GENMASK(1, 6) +#define AD7152_SETUP_RANGE_1pF GENMASK(2, 6) +#define AD7152_SETUP_RANGE_4pF GENMASK(3, 6) +#define AD7152_SETUP_RANGE(x) GENMASK(x, 6) /* Config Register Bit Designations (AD7152_REG_CFG) */ -#define AD7152_CONF_CH2EN (1 << 3) -#define AD7152_CONF_CH1EN (1 << 4) -#define AD7152_CONF_MODE_IDLE (0 << 0) -#define AD7152_CONF_MODE_CONT_CONV (1 << 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_CH2EN GENMASK(1, 3) +#define AD7152_CONF_CH1EN GENMASK(1, 4) +#define AD7152_CONF_MODE_IDLE GENMASK(0, 0) +#define AD7152_CONF_MODE_CONT_CONV GENMASK(1, 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 (1 << 7) -#define AD7152_CAPDAC_DACP(x) ((x) & 0x1F) +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) +#define AD7152_CAPDAC_DACP(x) GENMASK(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] 11+ messages in thread
* Re: [Outreachy kernel] [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-19 20:00 ` [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik @ 2017-02-19 20:04 ` Julia Lawall 2017-02-20 8:29 ` Lars-Peter Clausen 2017-02-20 9:48 ` Hendrik v. Raven 2 siblings, 0 replies; 11+ messages in thread From: Julia Lawall @ 2017-02-19 20:04 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 Mon, 20 Feb 2017, sayli karnik wrote: > Use GENMASK() macro for left shifting integers. > > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> It looks nicer this way :) Acked-by: Julia Lawall <julia.lawall@lip6.fr> > --- > v2: > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. > Removed extra parentheses around argument to macro > > drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c > index e8609b8..ab94fad 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -47,28 +47,28 @@ > #define AD7152_STATUS_PWDN BIT(7) > > /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ > -#define AD7152_SETUP_CAPDIFF (1 << 5) > -#define AD7152_SETUP_RANGE_2pF (0 << 6) > -#define AD7152_SETUP_RANGE_0_5pF (1 << 6) > -#define AD7152_SETUP_RANGE_1pF (2 << 6) > -#define AD7152_SETUP_RANGE_4pF (3 << 6) > -#define AD7152_SETUP_RANGE(x) ((x) << 6) > +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) > +#define AD7152_SETUP_RANGE_2pF GENMASK(0, 6) > +#define AD7152_SETUP_RANGE_0_5pF GENMASK(1, 6) > +#define AD7152_SETUP_RANGE_1pF GENMASK(2, 6) > +#define AD7152_SETUP_RANGE_4pF GENMASK(3, 6) > +#define AD7152_SETUP_RANGE(x) GENMASK(x, 6) > > /* Config Register Bit Designations (AD7152_REG_CFG) */ > -#define AD7152_CONF_CH2EN (1 << 3) > -#define AD7152_CONF_CH1EN (1 << 4) > -#define AD7152_CONF_MODE_IDLE (0 << 0) > -#define AD7152_CONF_MODE_CONT_CONV (1 << 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_CH2EN GENMASK(1, 3) > +#define AD7152_CONF_CH1EN GENMASK(1, 4) > +#define AD7152_CONF_MODE_IDLE GENMASK(0, 0) > +#define AD7152_CONF_MODE_CONT_CONV GENMASK(1, 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 (1 << 7) > -#define AD7152_CAPDAC_DACP(x) ((x) & 0x1F) > +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) > +#define AD7152_CAPDAC_DACP(x) GENMASK(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/b74094e5c94e11cad734b1bde11db0356e6d3aab.1487483384.git.karniksayli1995%40gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-19 20:00 ` [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik 2017-02-19 20:04 ` [Outreachy kernel] " Julia Lawall @ 2017-02-20 8:29 ` Lars-Peter Clausen 2017-02-20 9:48 ` Hendrik v. Raven 2 siblings, 0 replies; 11+ messages in thread From: Lars-Peter Clausen @ 2017-02-20 8:29 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/19/2017 09:00 PM, sayli karnik wrote: > Use GENMASK() macro for left shifting integers. > > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> > --- > v2: > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. > Removed extra parentheses around argument to macro > > drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c > index e8609b8..ab94fad 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -47,28 +47,28 @@ > #define AD7152_STATUS_PWDN BIT(7) > > /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ > -#define AD7152_SETUP_CAPDIFF (1 << 5) > [...] > +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) This was actually right, the first time, CAPDIFF is a single bit bitfield, so it should use BIT(). > [...] > -#define AD7152_CONF_CH2EN (1 << 3) > -#define AD7152_CONF_CH1EN (1 << 4) > [...] > +#define AD7152_CONF_CH2EN GENMASK(1, 3) > +#define AD7152_CONF_CH1EN GENMASK(1, 4) Same here CH2EN and CH1EN are single bit bitfields. > /* Capdac Register Bit Designations (AD7152_REG_CAPDAC_XXX) */ > -#define AD7152_CAPDAC_DACEN (1 << 7) > [...] > +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) Same for DACEN. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-19 20:00 ` [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik 2017-02-19 20:04 ` [Outreachy kernel] " Julia Lawall 2017-02-20 8:29 ` Lars-Peter Clausen @ 2017-02-20 9:48 ` Hendrik v. Raven 2017-02-20 10:40 ` [Outreachy kernel] " Julia Lawall 2 siblings, 1 reply; 11+ messages in thread From: Hendrik v. Raven @ 2017-02-20 9:48 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 Mon, Feb 20, 2017 at 01:30:27AM +0530, sayli karnik wrote: > Use GENMASK() macro for left shifting integers. > > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> > --- > v2: > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. > Removed extra parentheses around argument to macro > > drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c > index e8609b8..ab94fad 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -47,28 +47,28 @@ > #define AD7152_STATUS_PWDN BIT(7) > > /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ > -#define AD7152_SETUP_CAPDIFF (1 << 5) > -#define AD7152_SETUP_RANGE_2pF (0 << 6) > -#define AD7152_SETUP_RANGE_0_5pF (1 << 6) > -#define AD7152_SETUP_RANGE_1pF (2 << 6) > -#define AD7152_SETUP_RANGE_4pF (3 << 6) > -#define AD7152_SETUP_RANGE(x) ((x) << 6) > +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) > +#define AD7152_SETUP_RANGE_2pF GENMASK(0, 6) > +#define AD7152_SETUP_RANGE_0_5pF GENMASK(1, 6) > +#define AD7152_SETUP_RANGE_1pF GENMASK(2, 6) > +#define AD7152_SETUP_RANGE_4pF GENMASK(3, 6) > +#define AD7152_SETUP_RANGE(x) GENMASK(x, 6) I am a bit confused about the usage of GENMASK here. Unless I completely misunderstand the macro these are not at all equivalent to the previous bitshifts as most of them just expand to 0. > /* Config Register Bit Designations (AD7152_REG_CFG) */ > -#define AD7152_CONF_CH2EN (1 << 3) > -#define AD7152_CONF_CH1EN (1 << 4) > -#define AD7152_CONF_MODE_IDLE (0 << 0) > -#define AD7152_CONF_MODE_CONT_CONV (1 << 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_CH2EN GENMASK(1, 3) > +#define AD7152_CONF_CH1EN GENMASK(1, 4) > +#define AD7152_CONF_MODE_IDLE GENMASK(0, 0) > +#define AD7152_CONF_MODE_CONT_CONV GENMASK(1, 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 (1 << 7) > -#define AD7152_CAPDAC_DACP(x) ((x) & 0x1F) > +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) same here > +#define AD7152_CAPDAC_DACP(x) GENMASK(x, 0x1F) this is more something like ((x) & GENMASK(5,0)). > /* 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 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-20 9:48 ` Hendrik v. Raven @ 2017-02-20 10:40 ` Julia Lawall 2017-02-21 14:00 ` sayli karnik 0 siblings, 1 reply; 11+ messages in thread From: Julia Lawall @ 2017-02-20 10:40 UTC (permalink / raw) To: Hendrik v. Raven Cc: sayli karnik, outreachy-kernel, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio On Mon, 20 Feb 2017, Hendrik v. Raven wrote: > On Mon, Feb 20, 2017 at 01:30:27AM +0530, sayli karnik wrote: > > Use GENMASK() macro for left shifting integers. > > > > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> > > --- > > v2: > > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. > > Removed extra parentheses around argument to macro > > > > drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- > > 1 file changed, 16 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c > > index e8609b8..ab94fad 100644 > > --- a/drivers/staging/iio/cdc/ad7152.c > > +++ b/drivers/staging/iio/cdc/ad7152.c > > @@ -47,28 +47,28 @@ > > #define AD7152_STATUS_PWDN BIT(7) > > > > /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ > > -#define AD7152_SETUP_CAPDIFF (1 << 5) > > -#define AD7152_SETUP_RANGE_2pF (0 << 6) > > -#define AD7152_SETUP_RANGE_0_5pF (1 << 6) > > -#define AD7152_SETUP_RANGE_1pF (2 << 6) > > -#define AD7152_SETUP_RANGE_4pF (3 << 6) > > -#define AD7152_SETUP_RANGE(x) ((x) << 6) > > +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) > > +#define AD7152_SETUP_RANGE_2pF GENMASK(0, 6) > > +#define AD7152_SETUP_RANGE_0_5pF GENMASK(1, 6) > > +#define AD7152_SETUP_RANGE_1pF GENMASK(2, 6) > > +#define AD7152_SETUP_RANGE_4pF GENMASK(3, 6) > > +#define AD7152_SETUP_RANGE(x) GENMASK(x, 6) > I am a bit confused about the usage of GENMASK here. Unless I completely > misunderstand the macro these are not at all equivalent to the previous > bitshifts as most of them just expand to 0. Indeed. Sorry not to have been more attentive. The comment before the definition of GENMASK says: /* * Create a contiguous bitmask starting at bit position @l and ending at * position @h. For example * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. */ Here is an example of a commit that converts BIT calls to use GENMASK: 408fec3ff8 But it doesn't seem very convenient for the code modified here. julia > > > /* Config Register Bit Designations (AD7152_REG_CFG) */ > > -#define AD7152_CONF_CH2EN (1 << 3) > > -#define AD7152_CONF_CH1EN (1 << 4) > > -#define AD7152_CONF_MODE_IDLE (0 << 0) > > -#define AD7152_CONF_MODE_CONT_CONV (1 << 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_CH2EN GENMASK(1, 3) > > +#define AD7152_CONF_CH1EN GENMASK(1, 4) > > +#define AD7152_CONF_MODE_IDLE GENMASK(0, 0) > > +#define AD7152_CONF_MODE_CONT_CONV GENMASK(1, 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 (1 << 7) > > -#define AD7152_CAPDAC_DACP(x) ((x) & 0x1F) > > +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) > same here > > +#define AD7152_CAPDAC_DACP(x) GENMASK(x, 0x1F) > this is more something like ((x) & GENMASK(5,0)). > > > /* 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 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > 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/20170220094845.GA17071%40psyche.fritz.box. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-20 10:40 ` [Outreachy kernel] " Julia Lawall @ 2017-02-21 14:00 ` sayli karnik 2017-02-21 14:09 ` Julia Lawall 0 siblings, 1 reply; 11+ messages in thread From: sayli karnik @ 2017-02-21 14:00 UTC (permalink / raw) To: Julia Lawall Cc: Hendrik v. Raven, outreachy-kernel, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio On Mon, Feb 20, 2017 at 4:10 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > On Mon, 20 Feb 2017, Hendrik v. Raven wrote: > >> On Mon, Feb 20, 2017 at 01:30:27AM +0530, sayli karnik wrote: >> > Use GENMASK() macro for left shifting integers. >> > >> > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> >> > --- >> > v2: >> > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. >> > Removed extra parentheses around argument to macro >> > >> > drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- >> > 1 file changed, 16 insertions(+), 16 deletions(-) >> > >> > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c >> > index e8609b8..ab94fad 100644 >> > --- a/drivers/staging/iio/cdc/ad7152.c >> > +++ b/drivers/staging/iio/cdc/ad7152.c >> > @@ -47,28 +47,28 @@ >> > #define AD7152_STATUS_PWDN BIT(7) >> > >> > /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ >> > -#define AD7152_SETUP_CAPDIFF (1 << 5) >> > -#define AD7152_SETUP_RANGE_2pF (0 << 6) >> > -#define AD7152_SETUP_RANGE_0_5pF (1 << 6) >> > -#define AD7152_SETUP_RANGE_1pF (2 << 6) >> > -#define AD7152_SETUP_RANGE_4pF (3 << 6) >> > -#define AD7152_SETUP_RANGE(x) ((x) << 6) >> > +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) >> > +#define AD7152_SETUP_RANGE_2pF GENMASK(0, 6) >> > +#define AD7152_SETUP_RANGE_0_5pF GENMASK(1, 6) >> > +#define AD7152_SETUP_RANGE_1pF GENMASK(2, 6) >> > +#define AD7152_SETUP_RANGE_4pF GENMASK(3, 6) >> > +#define AD7152_SETUP_RANGE(x) GENMASK(x, 6) >> I am a bit confused about the usage of GENMASK here. Unless I completely >> misunderstand the macro these are not at all equivalent to the previous >> bitshifts as most of them just expand to 0. > Hello, Could you explain how these aren't equivalent? And should we drop the plan to use genmask here altogether? > Indeed. Sorry not to have been more attentive. The comment before the > definition of GENMASK says: > > /* > * Create a contiguous bitmask starting at bit position @l and ending at > * position @h. For example > * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. > */ > > Here is an example of a commit that converts BIT calls to use GENMASK: > > 408fec3ff8 > Okay so the above commit concludes that BIT(x) | BIT(y) equals GENMASK(x, y) But how do we use this for the code here? thanks, sayli > But it doesn't seem very convenient for the code modified here. > > julia > >> >> > /* Config Register Bit Designations (AD7152_REG_CFG) */ >> > -#define AD7152_CONF_CH2EN (1 << 3) >> > -#define AD7152_CONF_CH1EN (1 << 4) >> > -#define AD7152_CONF_MODE_IDLE (0 << 0) >> > -#define AD7152_CONF_MODE_CONT_CONV (1 << 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_CH2EN GENMASK(1, 3) >> > +#define AD7152_CONF_CH1EN GENMASK(1, 4) >> > +#define AD7152_CONF_MODE_IDLE GENMASK(0, 0) >> > +#define AD7152_CONF_MODE_CONT_CONV GENMASK(1, 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 (1 << 7) >> > -#define AD7152_CAPDAC_DACP(x) ((x) & 0x1F) >> > +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) >> same here >> > +#define AD7152_CAPDAC_DACP(x) GENMASK(x, 0x1F) >> this is more something like ((x) & GENMASK(5,0)). >> Sorry, my bad here! >> > /* 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 >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> -- >> 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/20170220094845.GA17071%40psyche.fritz.box. >> For more options, visit https://groups.google.com/d/optout. >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts 2017-02-21 14:00 ` sayli karnik @ 2017-02-21 14:09 ` Julia Lawall 0 siblings, 0 replies; 11+ messages in thread From: Julia Lawall @ 2017-02-21 14:09 UTC (permalink / raw) To: sayli karnik Cc: Julia Lawall, Hendrik v. Raven, outreachy-kernel, Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman, linux-iio On Tue, 21 Feb 2017, sayli karnik wrote: > On Mon, Feb 20, 2017 at 4:10 PM, Julia Lawall <julia.lawall@lip6.fr> wrote: > > > > > > On Mon, 20 Feb 2017, Hendrik v. Raven wrote: > > > >> On Mon, Feb 20, 2017 at 01:30:27AM +0530, sayli karnik wrote: > >> > Use GENMASK() macro for left shifting integers. > >> > > >> > Signed-off-by: sayli karnik <karniksayli1995@gmail.com> > >> > --- > >> > v2: > >> > Used GENMASK() macro instead of BIT() macro for multi-bit bitfields. > >> > Removed extra parentheses around argument to macro > >> > > >> > drivers/staging/iio/cdc/ad7152.c | 32 ++++++++++++++++---------------- > >> > 1 file changed, 16 insertions(+), 16 deletions(-) > >> > > >> > diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c > >> > index e8609b8..ab94fad 100644 > >> > --- a/drivers/staging/iio/cdc/ad7152.c > >> > +++ b/drivers/staging/iio/cdc/ad7152.c > >> > @@ -47,28 +47,28 @@ > >> > #define AD7152_STATUS_PWDN BIT(7) > >> > > >> > /* Setup Register Bit Designations (AD7152_REG_CHx_SETUP) */ > >> > -#define AD7152_SETUP_CAPDIFF (1 << 5) > >> > -#define AD7152_SETUP_RANGE_2pF (0 << 6) > >> > -#define AD7152_SETUP_RANGE_0_5pF (1 << 6) > >> > -#define AD7152_SETUP_RANGE_1pF (2 << 6) > >> > -#define AD7152_SETUP_RANGE_4pF (3 << 6) > >> > -#define AD7152_SETUP_RANGE(x) ((x) << 6) > >> > +#define AD7152_SETUP_CAPDIFF GENMASK(1, 5) > >> > +#define AD7152_SETUP_RANGE_2pF GENMASK(0, 6) > >> > +#define AD7152_SETUP_RANGE_0_5pF GENMASK(1, 6) > >> > +#define AD7152_SETUP_RANGE_1pF GENMASK(2, 6) > >> > +#define AD7152_SETUP_RANGE_4pF GENMASK(3, 6) > >> > +#define AD7152_SETUP_RANGE(x) GENMASK(x, 6) > >> I am a bit confused about the usage of GENMASK here. Unless I completely > >> misunderstand the macro these are not at all equivalent to the previous > >> bitshifts as most of them just expand to 0. > > > Hello, > Could you explain how these aren't equivalent? And should we drop the > plan to use genmask here altogether? Consider something like 5 << 6. 5 has the bit pattern 0101. There are not contiguous 1s in this bit pattern. So all you could do with GENMASK is make two one-bit masks and or them together. Or you could do the same with BIT. But probably neither is a big conceptual improvement over 5 << 6. So it seems that GENMASK is not so useful in this context, as far as I can see. julia > > > Indeed. Sorry not to have been more attentive. The comment before the > > definition of GENMASK says: > > > > /* > > * Create a contiguous bitmask starting at bit position @l and ending at > > * position @h. For example > > * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. > > */ > > > > Here is an example of a commit that converts BIT calls to use GENMASK: > > > > 408fec3ff8 > > > Okay so the above commit concludes that BIT(x) | BIT(y) equals GENMASK(x, y) > But how do we use this for the code here? > > thanks, > sayli > > But it doesn't seem very convenient for the code modified here. > > > > julia > > > >> > >> > /* Config Register Bit Designations (AD7152_REG_CFG) */ > >> > -#define AD7152_CONF_CH2EN (1 << 3) > >> > -#define AD7152_CONF_CH1EN (1 << 4) > >> > -#define AD7152_CONF_MODE_IDLE (0 << 0) > >> > -#define AD7152_CONF_MODE_CONT_CONV (1 << 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_CH2EN GENMASK(1, 3) > >> > +#define AD7152_CONF_CH1EN GENMASK(1, 4) > >> > +#define AD7152_CONF_MODE_IDLE GENMASK(0, 0) > >> > +#define AD7152_CONF_MODE_CONT_CONV GENMASK(1, 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 (1 << 7) > >> > -#define AD7152_CAPDAC_DACP(x) ((x) & 0x1F) > >> > +#define AD7152_CAPDAC_DACEN GENMASK(1, 7) > >> same here > >> > +#define AD7152_CAPDAC_DACP(x) GENMASK(x, 0x1F) > >> this is more something like ((x) & GENMASK(5,0)). > >> > Sorry, my bad here! > > >> > /* 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 > >> > > >> > -- > >> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > >> > the body of a message to majordomo@vger.kernel.org > >> > More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > >> -- > >> 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/20170220094845.GA17071%40psyche.fritz.box. > >> For more options, visit https://groups.google.com/d/optout. > >> > > -- > 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/CAKG5xWiqR95XiVz8zu_fR5kFdvjqYSDf1fEg_uDNDRax0Nqj%2Bw%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-02-25 16:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-19 19:57 [PATCH v2 0/2] Use BIT() and GENMASK() macros sayli karnik 2017-02-19 19:58 ` [PATCH v2 1/2] staging: iio: ad7152: Use BIT() macro for left shifting 1 sayli karnik 2017-02-19 20:02 ` [Outreachy kernel] " Julia Lawall 2017-02-25 16:47 ` Jonathan Cameron 2017-02-19 20:00 ` [PATCH v2 2/2] staging: iio: ad7152: Use GENMASK() macro for left shifts sayli karnik 2017-02-19 20:04 ` [Outreachy kernel] " Julia Lawall 2017-02-20 8:29 ` Lars-Peter Clausen 2017-02-20 9:48 ` Hendrik v. Raven 2017-02-20 10:40 ` [Outreachy kernel] " Julia Lawall 2017-02-21 14:00 ` sayli karnik 2017-02-21 14:09 ` Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).