* [PATCH] staging: iio: Replace bit shifting with BIT macro
@ 2017-09-17 1:11 Haneen Mohammed
2017-09-17 6:56 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 3+ messages in thread
From: Haneen Mohammed @ 2017-09-17 1:11 UTC (permalink / raw)
To: outreachy-kernel
Cc: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
Hartmut Knaack, Peter Meerwald-Stadler, Greg Kroah-Hartman
This patch replace bit shifting on 0, 1, 2, and 3 with BIT(x) macro.
Issue resolved with the following Coccinelle script:
@r1@
identifier x;
constant int g;
@@
(
0<<\(x\|g\)
|
1<<\(x\|g\)
|
2<<\(x\|g\)
|
3<<\(x\|g\)
)
@script:python b@
g2 <<r1.g;
y;
@@
coccinelle.y = int(g2) + 1
@c@
constant int r1.g;
identifier b.y;
@@
(
-(1 << g)
+BIT(g)
|
-(0 << g)
+ 0
|
-(2 << g)
+BIT(y)
|
-(3 << g)
+(BIT(y) | BIT(g))
)
Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 38 ++++++++++++-------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 3d539ee..83d1a92 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -35,32 +35,32 @@
#define AD5933_REG_IMAG_DATA 0x96 /* R, 2 bytes*/
/* AD5933_REG_CONTROL_HB Bits */
-#define AD5933_CTRL_INIT_START_FREQ (0x1 << 4)
-#define AD5933_CTRL_START_SWEEP (0x2 << 4)
-#define AD5933_CTRL_INC_FREQ (0x3 << 4)
+#define AD5933_CTRL_INIT_START_FREQ BIT(4)
+#define AD5933_CTRL_START_SWEEP BIT(5)
+#define AD5933_CTRL_INC_FREQ (BIT(5) | BIT(4))
#define AD5933_CTRL_REPEAT_FREQ (0x4 << 4)
#define AD5933_CTRL_MEASURE_TEMP (0x9 << 4)
#define AD5933_CTRL_POWER_DOWN (0xA << 4)
#define AD5933_CTRL_STANDBY (0xB << 4)
-#define AD5933_CTRL_RANGE_2000mVpp (0x0 << 1)
-#define AD5933_CTRL_RANGE_200mVpp (0x1 << 1)
-#define AD5933_CTRL_RANGE_400mVpp (0x2 << 1)
-#define AD5933_CTRL_RANGE_1000mVpp (0x3 << 1)
+#define AD5933_CTRL_RANGE_2000mVpp 0
+#define AD5933_CTRL_RANGE_200mVpp BIT(1)
+#define AD5933_CTRL_RANGE_400mVpp BIT(2)
+#define AD5933_CTRL_RANGE_1000mVpp (BIT(2) | BIT(1))
#define AD5933_CTRL_RANGE(x) ((x) << 1)
-#define AD5933_CTRL_PGA_GAIN_1 (0x1 << 0)
-#define AD5933_CTRL_PGA_GAIN_5 (0x0 << 0)
+#define AD5933_CTRL_PGA_GAIN_1 BIT(0)
+#define AD5933_CTRL_PGA_GAIN_5 0
/* AD5933_REG_CONTROL_LB Bits */
-#define AD5933_CTRL_RESET (0x1 << 4)
-#define AD5933_CTRL_INT_SYSCLK (0x0 << 3)
-#define AD5933_CTRL_EXT_SYSCLK (0x1 << 3)
+#define AD5933_CTRL_RESET BIT(4)
+#define AD5933_CTRL_INT_SYSCLK 0
+#define AD5933_CTRL_EXT_SYSCLK BIT(3)
/* AD5933_REG_STATUS Bits */
-#define AD5933_STAT_TEMP_VALID (0x1 << 0)
-#define AD5933_STAT_DATA_VALID (0x1 << 1)
-#define AD5933_STAT_SWEEP_DONE (0x1 << 2)
+#define AD5933_STAT_TEMP_VALID BIT(0)
+#define AD5933_STAT_DATA_VALID BIT(1)
+#define AD5933_STAT_SWEEP_DONE BIT(2)
/* I2C Block Commands */
#define AD5933_I2C_BLOCK_WRITE 0xA0
@@ -229,7 +229,7 @@ static int ad5933_set_freq(struct ad5933_state *st,
u8 d8[4];
} dat;
- freqreg = (u64) freq * (u64) (1 << 27);
+ freqreg = (u64)freq * (u64)BIT(27);
do_div(freqreg, st->mclk_hz / 4);
switch (reg) {
@@ -318,7 +318,7 @@ static ssize_t ad5933_show_frequency(struct device *dev,
freqreg = be32_to_cpu(dat.d32) & 0xFFFFFF;
freqreg = (u64)freqreg * (u64)(st->mclk_hz / 4);
- do_div(freqreg, 1 << 27);
+ do_div(freqreg, BIT(27));
return sprintf(buf, "%d\n", (int)freqreg);
}
@@ -452,9 +452,9 @@ static ssize_t ad5933_store(struct device *dev,
/* 2x, 4x handling, see datasheet */
if (val > 1022)
- val = (val >> 2) | (3 << 9);
+ val = (val >> 2) | (BIT(10) | BIT(9));
else if (val > 511)
- val = (val >> 1) | (1 << 9);
+ val = (val >> 1) | BIT(9);
dat = cpu_to_be16(val);
ret = ad5933_i2c_write(st->client,
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: iio: Replace bit shifting with BIT macro
2017-09-17 1:11 [PATCH] staging: iio: Replace bit shifting with BIT macro Haneen Mohammed
@ 2017-09-17 6:56 ` Julia Lawall
2017-09-17 18:30 ` Haneen Mohammed
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2017-09-17 6:56 UTC (permalink / raw)
To: Haneen Mohammed
Cc: outreachy-kernel, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
Greg Kroah-Hartman
On Sat, 16 Sep 2017, Haneen Mohammed wrote:
> This patch replace bit shifting on 0, 1, 2, and 3 with BIT(x) macro.
> Issue resolved with the following Coccinelle script:
>
> @r1@
> identifier x;
> constant int g;
> @@
> (
> 0<<\(x\|g\)
> |
> 1<<\(x\|g\)
> |
> 2<<\(x\|g\)
> |
> 3<<\(x\|g\)
> )
>
> @script:python b@
> g2 <<r1.g;
> y;
> @@
>
> coccinelle.y = int(g2) + 1
>
> @c@
> constant int r1.g;
> identifier b.y;
> @@
> (
> -(1 << g)
> +BIT(g)
> |
> -(0 << g)
> + 0
> |
> -(2 << g)
> +BIT(y)
> |
> -(3 << g)
> +(BIT(y) | BIT(g))
> )
Really good job on the semantic patch.
>
> Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> ---
> drivers/staging/iio/impedance-analyzer/ad5933.c | 38 ++++++++++++-------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 3d539ee..83d1a92 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -35,32 +35,32 @@
> #define AD5933_REG_IMAG_DATA 0x96 /* R, 2 bytes*/
>
> /* AD5933_REG_CONTROL_HB Bits */
> -#define AD5933_CTRL_INIT_START_FREQ (0x1 << 4)
> -#define AD5933_CTRL_START_SWEEP (0x2 << 4)
> -#define AD5933_CTRL_INC_FREQ (0x3 << 4)
> +#define AD5933_CTRL_INIT_START_FREQ BIT(4)
> +#define AD5933_CTRL_START_SWEEP BIT(5)
> +#define AD5933_CTRL_INC_FREQ (BIT(5) | BIT(4))
> #define AD5933_CTRL_REPEAT_FREQ (0x4 << 4)
> #define AD5933_CTRL_MEASURE_TEMP (0x9 << 4)
> #define AD5933_CTRL_POWER_DOWN (0xA << 4)
> #define AD5933_CTRL_STANDBY (0xB << 4)
I'm not sure that this is a good idea. Its more confusing than helpful to
have the different styles mixed up.
> -#define AD5933_CTRL_RANGE_2000mVpp (0x0 << 1)
> -#define AD5933_CTRL_RANGE_200mVpp (0x1 << 1)
> -#define AD5933_CTRL_RANGE_400mVpp (0x2 << 1)
> -#define AD5933_CTRL_RANGE_1000mVpp (0x3 << 1)
> +#define AD5933_CTRL_RANGE_2000mVpp 0
> +#define AD5933_CTRL_RANGE_200mVpp BIT(1)
> +#define AD5933_CTRL_RANGE_400mVpp BIT(2)
> +#define AD5933_CTRL_RANGE_1000mVpp (BIT(2) | BIT(1))
> #define AD5933_CTRL_RANGE(x) ((x) << 1)
>
> -#define AD5933_CTRL_PGA_GAIN_1 (0x1 << 0)
> -#define AD5933_CTRL_PGA_GAIN_5 (0x0 << 0)
> +#define AD5933_CTRL_PGA_GAIN_1 BIT(0)
> +#define AD5933_CTRL_PGA_GAIN_5 0
Again, this mixes styles. Even though 0 << 0 is pretty silly as
computation, it lines up well with what is above.
> /* AD5933_REG_CONTROL_LB Bits */
> -#define AD5933_CTRL_RESET (0x1 << 4)
> -#define AD5933_CTRL_INT_SYSCLK (0x0 << 3)
> -#define AD5933_CTRL_EXT_SYSCLK (0x1 << 3)
> +#define AD5933_CTRL_RESET BIT(4)
> +#define AD5933_CTRL_INT_SYSCLK 0
> +#define AD5933_CTRL_EXT_SYSCLK BIT(3)
Likewise.
> /* AD5933_REG_STATUS Bits */
> -#define AD5933_STAT_TEMP_VALID (0x1 << 0)
> -#define AD5933_STAT_DATA_VALID (0x1 << 1)
> -#define AD5933_STAT_SWEEP_DONE (0x1 << 2)
> +#define AD5933_STAT_TEMP_VALID BIT(0)
> +#define AD5933_STAT_DATA_VALID BIT(1)
> +#define AD5933_STAT_SWEEP_DONE BIT(2)
>
> /* I2C Block Commands */
> #define AD5933_I2C_BLOCK_WRITE 0xA0
> @@ -229,7 +229,7 @@ static int ad5933_set_freq(struct ad5933_state *st,
> u8 d8[4];
> } dat;
>
> - freqreg = (u64) freq * (u64) (1 << 27);
> + freqreg = (u64)freq * (u64)BIT(27);
> do_div(freqreg, st->mclk_hz / 4);
>
> switch (reg) {
> @@ -318,7 +318,7 @@ static ssize_t ad5933_show_frequency(struct device *dev,
> freqreg = be32_to_cpu(dat.d32) & 0xFFFFFF;
>
> freqreg = (u64)freqreg * (u64)(st->mclk_hz / 4);
> - do_div(freqreg, 1 << 27);
> + do_div(freqreg, BIT(27));
>
> return sprintf(buf, "%d\n", (int)freqreg);
> }
> @@ -452,9 +452,9 @@ static ssize_t ad5933_store(struct device *dev,
>
> /* 2x, 4x handling, see datasheet */
> if (val > 1022)
> - val = (val >> 2) | (3 << 9);
> + val = (val >> 2) | (BIT(10) | BIT(9));
This would be better without the parentheses on (BIT(10) | BIT(9)
julia
> else if (val > 511)
> - val = (val >> 1) | (1 << 9);
> + val = (val >> 1) | BIT(9);
>
> dat = cpu_to_be16(val);
> ret = ad5933_i2c_write(st->client,
> --
> 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/20170917011142.GA26676%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: iio: Replace bit shifting with BIT macro
2017-09-17 6:56 ` [Outreachy kernel] " Julia Lawall
@ 2017-09-17 18:30 ` Haneen Mohammed
0 siblings, 0 replies; 3+ messages in thread
From: Haneen Mohammed @ 2017-09-17 18:30 UTC (permalink / raw)
To: Julia Lawall
Cc: outreachy-kernel, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Hartmut Knaack, Peter Meerwald-Stadler,
Greg Kroah-Hartman
On Sun, Sep 17, 2017 at 08:56:40AM +0200, Julia Lawall wrote:
>
>
> On Sat, 16 Sep 2017, Haneen Mohammed wrote:
>
> > This patch replace bit shifting on 0, 1, 2, and 3 with BIT(x) macro.
> > Issue resolved with the following Coccinelle script:
> >
> > @r1@
> > identifier x;
> > constant int g;
> > @@
> > (
> > 0<<\(x\|g\)
> > |
> > 1<<\(x\|g\)
> > |
> > 2<<\(x\|g\)
> > |
> > 3<<\(x\|g\)
> > )
> >
> > @script:python b@
> > g2 <<r1.g;
> > y;
> > @@
> >
> > coccinelle.y = int(g2) + 1
> >
> > @c@
> > constant int r1.g;
> > identifier b.y;
> > @@
> > (
> > -(1 << g)
> > +BIT(g)
> > |
> > -(0 << g)
> > + 0
> > |
> > -(2 << g)
> > +BIT(y)
> > |
> > -(3 << g)
> > +(BIT(y) | BIT(g))
> > )
>
> Really good job on the semantic patch.
>
>
Thank you! I remember you helped me with it two years ago :)
> >
> > Signed-off-by: Haneen Mohammed <hamohammed.sa@gmail.com>
> > ---
> > drivers/staging/iio/impedance-analyzer/ad5933.c | 38 ++++++++++++-------------
> > 1 file changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > index 3d539ee..83d1a92 100644
> > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > @@ -35,32 +35,32 @@
> > #define AD5933_REG_IMAG_DATA 0x96 /* R, 2 bytes*/
> >
> > /* AD5933_REG_CONTROL_HB Bits */
> > -#define AD5933_CTRL_INIT_START_FREQ (0x1 << 4)
> > -#define AD5933_CTRL_START_SWEEP (0x2 << 4)
> > -#define AD5933_CTRL_INC_FREQ (0x3 << 4)
> > +#define AD5933_CTRL_INIT_START_FREQ BIT(4)
> > +#define AD5933_CTRL_START_SWEEP BIT(5)
> > +#define AD5933_CTRL_INC_FREQ (BIT(5) | BIT(4))
> > #define AD5933_CTRL_REPEAT_FREQ (0x4 << 4)
> > #define AD5933_CTRL_MEASURE_TEMP (0x9 << 4)
> > #define AD5933_CTRL_POWER_DOWN (0xA << 4)
> > #define AD5933_CTRL_STANDBY (0xB << 4)
>
> I'm not sure that this is a good idea. Its more confusing than helpful to
> have the different styles mixed up.
>
>
I see. I wasn't sure how to convert the others so I left them.
I'll remove the bit macro from #define then.
Thank you!
Haneen
> > -#define AD5933_CTRL_RANGE_2000mVpp (0x0 << 1)
> > -#define AD5933_CTRL_RANGE_200mVpp (0x1 << 1)
> > -#define AD5933_CTRL_RANGE_400mVpp (0x2 << 1)
> > -#define AD5933_CTRL_RANGE_1000mVpp (0x3 << 1)
> > +#define AD5933_CTRL_RANGE_2000mVpp 0
> > +#define AD5933_CTRL_RANGE_200mVpp BIT(1)
> > +#define AD5933_CTRL_RANGE_400mVpp BIT(2)
> > +#define AD5933_CTRL_RANGE_1000mVpp (BIT(2) | BIT(1))
> > #define AD5933_CTRL_RANGE(x) ((x) << 1)
> >
> > -#define AD5933_CTRL_PGA_GAIN_1 (0x1 << 0)
> > -#define AD5933_CTRL_PGA_GAIN_5 (0x0 << 0)
> > +#define AD5933_CTRL_PGA_GAIN_1 BIT(0)
> > +#define AD5933_CTRL_PGA_GAIN_5 0
>
> Again, this mixes styles. Even though 0 << 0 is pretty silly as
> computation, it lines up well with what is above.
>
> > /* AD5933_REG_CONTROL_LB Bits */
> > -#define AD5933_CTRL_RESET (0x1 << 4)
> > -#define AD5933_CTRL_INT_SYSCLK (0x0 << 3)
> > -#define AD5933_CTRL_EXT_SYSCLK (0x1 << 3)
> > +#define AD5933_CTRL_RESET BIT(4)
> > +#define AD5933_CTRL_INT_SYSCLK 0
> > +#define AD5933_CTRL_EXT_SYSCLK BIT(3)
>
> Likewise.
>
> > /* AD5933_REG_STATUS Bits */
> > -#define AD5933_STAT_TEMP_VALID (0x1 << 0)
> > -#define AD5933_STAT_DATA_VALID (0x1 << 1)
> > -#define AD5933_STAT_SWEEP_DONE (0x1 << 2)
> > +#define AD5933_STAT_TEMP_VALID BIT(0)
> > +#define AD5933_STAT_DATA_VALID BIT(1)
> > +#define AD5933_STAT_SWEEP_DONE BIT(2)
> >
> > /* I2C Block Commands */
> > #define AD5933_I2C_BLOCK_WRITE 0xA0
> > @@ -229,7 +229,7 @@ static int ad5933_set_freq(struct ad5933_state *st,
> > u8 d8[4];
> > } dat;
> >
> > - freqreg = (u64) freq * (u64) (1 << 27);
> > + freqreg = (u64)freq * (u64)BIT(27);
> > do_div(freqreg, st->mclk_hz / 4);
> >
> > switch (reg) {
> > @@ -318,7 +318,7 @@ static ssize_t ad5933_show_frequency(struct device *dev,
> > freqreg = be32_to_cpu(dat.d32) & 0xFFFFFF;
> >
> > freqreg = (u64)freqreg * (u64)(st->mclk_hz / 4);
> > - do_div(freqreg, 1 << 27);
> > + do_div(freqreg, BIT(27));
> >
> > return sprintf(buf, "%d\n", (int)freqreg);
> > }
> > @@ -452,9 +452,9 @@ static ssize_t ad5933_store(struct device *dev,
> >
> > /* 2x, 4x handling, see datasheet */
> > if (val > 1022)
> > - val = (val >> 2) | (3 << 9);
> > + val = (val >> 2) | (BIT(10) | BIT(9));
>
> This would be better without the parentheses on (BIT(10) | BIT(9)
>
> julia
>
> > else if (val > 511)
> > - val = (val >> 1) | (1 << 9);
> > + val = (val >> 1) | BIT(9);
> >
> > dat = cpu_to_be16(val);
> > ret = ad5933_i2c_write(st->client,
> > --
> > 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/20170917011142.GA26676%40Haneen.
> > For more options, visit https://groups.google.com/d/optout.
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-09-17 18:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-17 1:11 [PATCH] staging: iio: Replace bit shifting with BIT macro Haneen Mohammed
2017-09-17 6:56 ` [Outreachy kernel] " Julia Lawall
2017-09-17 18:30 ` Haneen Mohammed
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.