Re: [libgpiod][PATCH] core: Basic port to uAPI v2

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Fri, Oct 2, 2020 at 9:32 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> Port existing implementation from GPIO uAPI v1 to v2.
> The libgpiod external interface remains unchanged, only the internal
> implementation switches from uAPI v1 to v2.

Cool!

> This is a minimal port - uAPI v2 features are only used where it
> simplifies the implementation, specifically multiple events on a bulk can
> now be handled directly by the kernel in a single v2 line request rather
> than being emulated by multiple v1 event requests.

...

> +       if (config->flags & GPIOD_LINE_REQUEST_FLAG_BIAS_DISABLE)
> +               bias_flags++;
> +       if (config->flags & GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_UP)
> +               bias_flags++;
> +       if (config->flags & GPIOD_LINE_REQUEST_FLAG_BIAS_PULL_DOWN)
> +               bias_flags++;
> +       if (bias_flags > 1)
> +               return false;

Seems to me like an emulation of hweight(), although I don't know if
we have in standard libraries such.

...

> -       if (values) {
> -               for (i = 0; i < gpiod_line_bulk_num_lines(bulk); i++)
> -                       data.values[i] = (uint8_t)!!values[i];
> +       for (i = 0; i < gpiod_line_bulk_num_lines(bulk); i++) {
> +               lines_bitmap_set_bit(&lv.mask, i);
> +               lines_bitmap_assign_bit(&lv.bits, i, values && values[i]);
>         }

Hmm... What about
       for (i = 0; i < gpiod_line_bulk_num_lines(bulk); i++)
               lines_bitmap_set_bit(&lv.mask, i);

  if (values) {
       for (i = 0; i < gpiod_line_bulk_num_lines(bulk); i++)
               lines_bitmap_assign_bit(&lv.bits, i, values[i]);
  }

?

...

>         /*
>          * 16 is the maximum number of events the kernel can store in the FIFO
>          * so we can allocate the buffer on the stack.
> +        *
> +        * NOTE: This is no longer strictly true for uAPI v2.  While 16 is
> +        * the default for single line, a request with multiple lines will

for a single

> +        * have a larger buffer.  So need to rethink the allocation here,

So we (I, ...?) need

> +        * or at least the comment above...
>          */

-- 
With Best Regards,
Andy Shevchenko