2019-04-03 20:46:26

by Lucas Oshiro

Subject: [PATCH] staging: iio: cdc: ad7746: Replace bitshift by BIT

Replace bitshifts on lines 54, 56 and 78 of ad7746.c.

Signed-off-by: Lucas Oshiro <[email protected]>
---
drivers/staging/iio/cdc/ad7746.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 0eb28fea876e..ea48b14cee72 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -51,9 +51,9 @@
#define AD7746_CAPSETUP_CACHOP BIT(0)

/* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
-#define AD7746_VTSETUP_VTEN (1 << 7)
+#define AD7746_VTSETUP_VTEN BIT(7)
#define AD7746_VTSETUP_VTMD_INT_TEMP (0 << 5)
-#define AD7746_VTSETUP_VTMD_EXT_TEMP (1 << 5)
+#define AD7746_VTSETUP_VTMD_EXT_TEMP BIT(5)
#define AD7746_VTSETUP_VTMD_VDD_MON (2 << 5)
#define AD7746_VTSETUP_VTMD_EXT_VIN (3 << 5)
#define AD7746_VTSETUP_EXTREF BIT(4)
@@ -75,7 +75,7 @@
#define AD7746_CONF_VTFS_MASK GENMASK(7, 6)
#define AD7746_CONF_CAPFS_MASK GENMASK(5, 3)
#define AD7746_CONF_MODE_IDLE (0 << 0)
-#define AD7746_CONF_MODE_CONT_CONV (1 << 0)
+#define AD7746_CONF_MODE_CONT_CONV BIT(0)
#define AD7746_CONF_MODE_SINGLE_CONV (2 << 0)
#define AD7746_CONF_MODE_PWRDN (3 << 0)
#define AD7746_CONF_MODE_OFFS_CAL (5 << 0)
--
2.21.0

2019-04-04 06:42:37

by Alexandru Ardelean

Subject: Re: [PATCH] staging: iio: cdc: ad7746: Replace bitshift by BIT

On Wed, Apr 3, 2019 at 11:46 PM Lucas Oshiro <[email protected]> wrote:
>
> Replace bitshifts on lines 54, 56 and 78 of ad7746.c.
>

Hey,
This is only partially done.
If doing conversions to BIT(x) macro, I would say to do them for all cases.

Thanks
Alex

> Signed-off-by: Lucas Oshiro <[email protected]>
> ---
> drivers/staging/iio/cdc/ad7746.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
> index 0eb28fea876e..ea48b14cee72 100644
> --- a/drivers/staging/iio/cdc/ad7746.c
> +++ b/drivers/staging/iio/cdc/ad7746.c
> @@ -51,9 +51,9 @@
> #define AD7746_CAPSETUP_CACHOP BIT(0)
>
> /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
> -#define AD7746_VTSETUP_VTEN (1 << 7)
> +#define AD7746_VTSETUP_VTEN BIT(7)
> #define AD7746_VTSETUP_VTMD_INT_TEMP (0 << 5)
> -#define AD7746_VTSETUP_VTMD_EXT_TEMP (1 << 5)
> +#define AD7746_VTSETUP_VTMD_EXT_TEMP BIT(5)
> #define AD7746_VTSETUP_VTMD_VDD_MON (2 << 5)
> #define AD7746_VTSETUP_VTMD_EXT_VIN (3 << 5)
> #define AD7746_VTSETUP_EXTREF BIT(4)
> @@ -75,7 +75,7 @@
> #define AD7746_CONF_VTFS_MASK GENMASK(7, 6)
> #define AD7746_CONF_CAPFS_MASK GENMASK(5, 3)
> #define AD7746_CONF_MODE_IDLE (0 << 0)
> -#define AD7746_CONF_MODE_CONT_CONV (1 << 0)
> +#define AD7746_CONF_MODE_CONT_CONV BIT(0)
> #define AD7746_CONF_MODE_SINGLE_CONV (2 << 0)
> #define AD7746_CONF_MODE_PWRDN (3 << 0)
> #define AD7746_CONF_MODE_OFFS_CAL (5 << 0)
> --
> 2.21.0
>

2019-04-04 09:14:53

by Dan Carpenter

Subject: Re: [PATCH] staging: iio: cdc: ad7746: Replace bitshift by BIT

On Wed, Apr 03, 2019 at 05:45:02PM -0300, Lucas Oshiro wrote:
> #define AD7746_VTSETUP_VTMD_INT_TEMP (0 << 5)
> -#define AD7746_VTSETUP_VTMD_EXT_TEMP (1 << 5)
> +#define AD7746_VTSETUP_VTMD_EXT_TEMP BIT(5)

No, the original is more readable. Otherwise you can't see that it's
part of a set.

Just ignore checkpatch when it gives nonsense advice.

regards,
dan carpenter

2019-04-08 12:36:23

by Joe Perches

Subject: Re: [PATCH] staging: iio: cdc: ad7746: Replace bitshift by BIT

On Wed, 2019-04-03 at 17:45 -0300, Lucas Oshiro wrote:
> Replace bitshifts on lines 54, 56 and 78 of ad7746.c.

checkpatch is not something that should be followed
blindly. Look at the shifted blocks and determine if
you think it was better before your proposed change.

> diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
[]
> @@ -51,9 +51,9 @@
> #define AD7746_CAPSETUP_CACHOP BIT(0)
>
> /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
> -#define AD7746_VTSETUP_VTEN (1 << 7)
> +#define AD7746_VTSETUP_VTEN BIT(7)
> #define AD7746_VTSETUP_VTMD_INT_TEMP (0 << 5)
> -#define AD7746_VTSETUP_VTMD_EXT_TEMP (1 << 5)
> +#define AD7746_VTSETUP_VTMD_EXT_TEMP BIT(5)
> #define AD7746_VTSETUP_VTMD_VDD_MON (2 << 5)
> #define AD7746_VTSETUP_VTMD_EXT_VIN (3 << 5)
> #define AD7746_VTSETUP_EXTREF BIT(4)
> @@ -75,7 +75,7 @@
> #define AD7746_CONF_VTFS_MASK GENMASK(7, 6)
> #define AD7746_CONF_CAPFS_MASK GENMASK(5, 3)
> #define AD7746_CONF_MODE_IDLE (0 << 0)
> -#define AD7746_CONF_MODE_CONT_CONV (1 << 0)
> +#define AD7746_CONF_MODE_CONT_CONV BIT(0)
> #define AD7746_CONF_MODE_SINGLE_CONV (2 << 0)
> #define AD7746_CONF_MODE_PWRDN (3 << 0)
> #define AD7746_CONF_MODE_OFFS_CAL (5 << 0)

Now the code looks unbalanced.