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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jan 2022 18:55:50 +0200
From:   Cosmin Tanislav <demonsingur@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     cosmin.tanislav@...log.com, Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Rob Herring <robh+dt@...nel.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH 3/3] iio: addac: ad74413r: correct comparator gpio getters
 mask usage



On 1/9/22 14:13, Andy Shevchenko wrote:
> On Fri, Jan 7, 2022 at 7:34 AM Cosmin Tanislav <demonsingur@...il.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?