diff mbox series

[3/3] iio: addac: ad74413r: correct comparator gpio getters mask usage

Message ID 20220106062255.3208817-3-cosmin.tanislav@analog.com
State Superseded
Headers show
Series [1/3] iio: addac: ad74413r: use ngpio size when iterating over mask | expand

Commit Message

Cosmin Tanislav Jan. 6, 2022, 6:22 a.m. UTC
The value of the GPIOs is currently altered using offsets rather
than masks. Make use the BIT macro to turn the offsets into masks.

Signed-off-by: Cosmin Tanislav <cosmin.tanislav@analog.com>
---
 drivers/iio/addac/ad74413r.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Andy Shevchenko Jan. 9, 2022, 12:13 p.m. UTC | #1
On Fri, Jan 7, 2022 at 7:34 AM Cosmin Tanislav <demonsingur@gmail.com> wrote:
>
> The value of the GPIOs is currently altered using offsets rather
> than masks. Make use the BIT macro to turn the offsets into masks.

of the

...

> -       status &= AD74413R_DIN_COMP_OUT_SHIFT_X(real_offset);
> +       status &= BIT(real_offset);

But this is completely different.

> +       bitmap_zero(bits, chip->ngpio);
> +
>         for_each_set_bit(offset, mask, chip->ngpio) {
>                 unsigned int real_offset = st->comp_gpio_offsets[offset];
>
>                 if (val & BIT(real_offset))
> -                       *bits |= offset;
> +                       *bits |= BIT(offset);

So, how was it working before? If it fixes, it should go with the
Fixes tag and before patch 2.

>         }

On top of that, you may try to see if one of bitmap_*() APIs can be
suitable here to perform the above in a more optimal way.
(At least this conditional can be replaced with __asign_bit() call,
but I think refactoring the entire loop may reveal a better approach)
diff mbox series

Patch

diff --git a/drivers/iio/addac/ad74413r.c b/drivers/iio/addac/ad74413r.c
index 3d089c0b6f7a..a69dee667441 100644
--- a/drivers/iio/addac/ad74413r.c
+++ b/drivers/iio/addac/ad74413r.c
@@ -134,7 +134,6 @@  struct ad74413r_state {
 #define AD74413R_CH_EN_MASK(x)		BIT(x)
 
 #define AD74413R_REG_DIN_COMP_OUT		0x25
-#define AD74413R_DIN_COMP_OUT_SHIFT_X(x)	x
 
 #define AD74413R_REG_ADC_RESULT_X(x)	(0x26 + (x))
 #define AD74413R_ADC_RESULT_MAX		GENMASK(15, 0)
@@ -316,7 +315,7 @@  static int ad74413r_gpio_get(struct gpio_chip *chip, unsigned int offset)
 	if (ret)
 		return ret;
 
-	status &= AD74413R_DIN_COMP_OUT_SHIFT_X(real_offset);
+	status &= BIT(real_offset);
 
 	return status ? 1 : 0;
 }
@@ -334,11 +333,13 @@  static int ad74413r_gpio_get_multiple(struct gpio_chip *chip,
 	if (ret)
 		return ret;
 
+	bitmap_zero(bits, chip->ngpio);
+
 	for_each_set_bit(offset, mask, chip->ngpio) {
 		unsigned int real_offset = st->comp_gpio_offsets[offset];
 
 		if (val & BIT(real_offset))
-			*bits |= offset;
+			*bits |= BIT(offset);
 	}
 
 	return ret;