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

[Cosmin Tanislav <demonsingur@gmail.com>]

On 1/9/22 14:13, Andy Shevchenko wrote:
> 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.

What do you mean by this is completely different?

It was broken before, it is fixed now. Indeed, I'm missing
the Fixes tag, if that's what you meant.

> 
>> +       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)
> 

I can replace the if and bitmap_zero with __assign_bit, as you
suggested. I'm not familiar with bitmap APIs, do you have a suggestion?