linux-kernel - Re: [PATCH v12 4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set

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:   Tue, 10 Nov 2020 11:02:43 +0100
From:   Michal Simek <michal.simek@...inx.com>
To:     William Breathitt Gray <vilhelm.gray@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC:     Syed Nayyar Waris <syednwaris@...il.com>,
        Arnd Bergmann <arnd@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Michal Simek <michal.simek@...inx.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v12 4/4] gpio: xilinx: Utilize generic bitmap_get_value
 and _set_value



On 09. 11. 20 18:31, William Breathitt Gray wrote:
> On Mon, Nov 09, 2020 at 07:22:20PM +0200, Andy Shevchenko wrote:
>> On Mon, Nov 09, 2020 at 12:11:40PM -0500, William Breathitt Gray wrote:
>>> On Mon, Nov 09, 2020 at 10:15:29PM +0530, Syed Nayyar Waris wrote:
>>>> On Mon, Nov 09, 2020 at 03:41:53PM +0100, Arnd Bergmann wrote:
>>
>> ...
>>
>>>>  static inline void bitmap_set_value(unsigned long *map,
>>>> -                                    unsigned long value,
>>>> +                                    unsigned long value, const size_t length,
>>>>                                      unsigned long start, unsigned long nbits)
>>>>  {
>>>>          const size_t index = BIT_WORD(start);
>>>> @@ -15,6 +15,10 @@ static inline void bitmap_set_value(unsigned long *map,
>>>>          } else {
>>>>                  map[index + 0] &= ~BITMAP_FIRST_WORD_MASK(start);
>>>>                  map[index + 0] |= value << offset;
>>>> +
>>>> +               if (index + 1 >= length)
>>>> +                       __builtin_unreachable();
>>>> +
>>>>                  map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
>>>>                  map[index + 1] |= value >> space;
>>>>          }
>>>
>>> Hi Syed,
>>>
>>> Let's rename 'length' to 'nbits' as Arnd suggested, and rename 'nbits'
>>> to value_width.
>>
>> length here is in longs. I guess this is the point of entire patch.
> 
> Ah yes, this should become 'const unsigned long nbits' and represent the
> length of the bitmap in bits and not longs.
> 
>> But to me sounds like it would be better to have simply bitmap_set_value64() /
>> bitmap_set_value32() with proper optimization done and forget about variadic
>> ones for now.
> 
> The gpio-xilinx driver can have arbitrary sizes for width[0] and
> width[1], so unfortunately that means we don't know the start position
> nor the width of the value beforehand.

Start position should be all the time zero. You can't configure this IP
to start from bit 2. Width can vary but start is IMHO all the time from
0 bit.

Thanks,
Michal