[libgpiod] libgpiod v2.0 API discussion

[Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>]

Hello!

I'd like to start a discussion on the new API in libgpiod v2.0. Below
are my ideas for different parts starting from the top of
include/gpiod.h. I'm open for any other ideas.

Arnd: I'm Cc'ing you mostly for your knowledge on the year 2036
problem: please take a look at the part about reading line events and
whether we should use struct timespec in userspace or if it's
discouraged.

* Context-less API

I think we can entirely remove it. As I mentioned in another thread: I
have never received a single question about it - as opposed to all the
other interfaces - so I suppose it's just needless cruft.

* Chip properties and handling

I think we're mostly good on that front. V2 has not altered that part.

* Line watch

Some preliminary work was done by me for V1 watch but it never got
into mainline libgpiod. Since the v2 of line watch uAPI mostly follows
what I did for v1 (except for a different lineinfo struct) I think we
can mostly adapt the code from my development branch[1].

New tool: gpiowatch will be provided in tools/.

* Line properties

We're mostly fine here, but I'd still remove
gpiod_line_is_open_source/drain() in favor of an enum defining drive
flags and a function called gpiod_line_drive(). Also:
gpiod_line_active_state() should probably become
gpiod_line_is_active_low() and we can drop the corresponding enum.

* Line bulks

Already submitted a patch that makes gpiod_line_bulk opaque across the
entire codebase.

* Line lookup by name

One thing that was omitted in the first version was the fact that GPIO
line names are not unique in the kernel. Functions that do name lookup
from a single chip should return bulk objects. Functions that can
lookup several lines with different names should probably be removed
because it's difficult to design an elegant API that would take this
non-uniqueness into account. Returning arrays of bulks sounds like a
bad idea.

For global line lookup: the core C library should probably offer some
function to which a callback would be passed. This would then be
called every time a matching line was found and would take the
matching line and the owning chip as arguments.

gpiofind tool would need to be updated and display multiple lines if
more lines are matching.

Testing of this part will only be possible once we have the new
configfs-based gpio-mockup module on which I'm working currently. It
will provide a way to flexibly name separate dummy lines.

* Iterators

Kent suggests we ditch iterators entirely. I agree for line bulks -
seems like the overhead of allocating an iterator is not worth it
(except for bindings where we use the built-in language features of
C++ and Python).

Kent also pointed out that the chip iterator will fail if opening any
of the chips fails (which can happen for instance if the user only has
permissions to access certain chips, not all of them) and this needs
addressing.

I'd still prefer to keep some way of detecting chips in the system -
be it an iterator or a loop function taking a callback as argument.
Now that we have the is_gpiochip_cdev() function in the library, I
think we don't have to limit ourselves to only checking devices whose
names start with 'gpiochip' - we can check all files in /dev and
verify in /sys if they're GPIO chips or not. Then the iterator would
only iterate over chips we managed to access. Let me know what you
think.

I think that is_gpiochip_cdev() could be renamed to
gpiod_is_gpiochip_cdev() and exported so that users can verify on
their own if given device file is a GPIO chip.

I think Kent suggested returning an array of paths to GPIO chips too.

I'm considering dropping the line iterator as we can simply call
gpiod_chip_get_all_lines() and loop over the returned bulk.

* Reading line events

Since we now can have a single file descriptor for multiple monitored
lines - reading of the events needs to be modified. Event structure
needs to reference the line object with which it's associated. Adding
additional fields for the seq numbers is a no-brainer of course.

I'm wondering if we should still be using struct timespec for
timestamps. On one hand it's used everywhere in libc but it's also
subject to ABI change due to year 2036 problem. Maybe we should switch
to nanoseconds in 64-bit integer just like what we get from the kernel
but then user-space won't have standard tools to deal with such
timestamps.

Some other general design issues I'd like to discuss too:

* Opaque data types

Kent suggested we make all user-facing data types opaque. While I
agree for almost all structs - I think that line & watch events as
well as request config should remain visible to users (although with
padding this time - I missed this in v1.x).

For events the reason is simple: with opaque types we'd face an
overhead with malloc() and I think it's important that we can read
them as fast as possible.

For config: I believe an opaque request config structure will require
a lot of getter/setter boiler plate code for not much profit.

Let's discuss!

Best Regards,
Bartosz Golaszewski

[1] https://github.com/brgl/libgpiod/commits/topic/line-watch