Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support uAPI v2 #68

Open
hellow554 opened this issue Jan 27, 2022 · 13 comments
Open

Support uAPI v2 #68

hellow554 opened this issue Jan 27, 2022 · 13 comments
Labels
help wanted Extra attention is needed

Comments

@hellow554
Copy link
Contributor

torvalds/linux@b53911a introduces the uapi v2 which has slightly different ioctl calls. Also v1 is deprecated from that point on: torvalds/linux@b234d23

@eldruin eldruin added the help wanted Extra attention is needed label Jan 27, 2022
@fpagliughi
Copy link
Contributor

I'm on it!

Here's what I'm thinking...

  1. Clean up what we have now and bump the version to 1.0 to keep consistent with the kernel versioning. Make a v1.x branch in case we want to maintain some small fixes for it going forward. (Meaning maintain a v1.x Rust crate for a little while)
  2. Start developing master toward the new v2 kernel API. When ready, we'll release the crate as version 2.0, again to keep consistent with the kernel numbering.

In addition, I need to add async support for smol/async-std, since my primary customer uses those in their code. I would probably do that first to get it in the v1 branch going forward.

How does that sound?

@eldruin
Copy link
Member

eldruin commented Apr 8, 2022

Cool to hear!
Regarding the versioning, I would like to propose something different:

  1. Keep the versioning of this crate independent from that of the kernel API being used. This is the only way to ensure we can abide by Semantic Versioning in this crate. Otherwise, we would never be able to release a breaking change in the iterface of this crate after 1.0 is out. e.g. which kernel API version does gpio-cdev 3.0 use? Also, releasing breaking changes in a minor version would be a no-go.
  2. Implementation-wise, depending on how technically feasible this would be, how about defining 2 modules, one for each kernel API version? With this we could do things like:
    1. If the API version choice can be hidden from the user, we can let the user choose via cargo features but keep the same interface. Alternatively via top-level modules as a sort of "namespace".
    2. If the API version choice cannot be hidden, for example, we could expose only the v2 stuff and keep v1 in a submodule (or viceversa), also even optionally gated on a cargo feature.

What do you think?

@ryankurte
Copy link

ryankurte commented Apr 11, 2022

iiinteresting, i'd echo @eldriun's thoughts, this seems neat but also like a bit of a compatibility problem / not something most users should need to care about... would it be reasonable to detect and abstract this at runtime as the default?

@fpagliughi
Copy link
Contributor

fpagliughi commented Apr 12, 2022

Yeah, looking at it more closely, I think you guys are right.

It seems that, looking at the new header file, mostly v2 types and ioctl's were added rather than modifying the existing API, which I misunderstood when I read up on the new features. So it seems that existing v1 apps still work, although there is the notable difference that the event timestamps are now from the monotonic clock, reporting the number of seconds since the board booted (or so it seems on an RPi) rather than a time_t value.

Given that both API's are still supported side-by-side, then my idea of maintaining separate versions of this crate doesn't make sense.

But keeping with the way it was laid out in the kernel, perhaps we should follow along, and rather than try to abstract the two versions, just add support for v2 and let the application writer decide which to implement.

@fpagliughi
Copy link
Contributor

Apologies for the slow start on this. I ordered a couple Raspberry Pi's to develop/test these new features... and now I have to acknowledge they they will never actually arrive!

I'll see if I can borrow a board from somewhere or seek an alternate target. Any ideas for a board that has a new kernel with v2 out of the box?

@alexferro
Copy link

Just as a note from a user of this crate here. I'd appreciate a note in the docs that the timestamps have changed clock base as of v5.7, even with the old v1 ABI as used and documented in the current crate version. While for my purposes (and probably most) CLOCK_MONOTONIC is better than CLOCK_REALTIME anyway, and none of this is your fault, I was very confused today while writing code, and looking for my bug when I kept getting times in 1970 coming out of the string formatting.
Of course, I'm not really sure how updating the docs only works as a crate publisher, so it may have to just be something that's fixed in the version when you also support the v2 ABI.

Also, it looks like v5.11 added a flag to select between the two clocks in the v2 ABI. That might be a useful feature to support.

https://docs.rs/gpio-cdev/latest/gpio_cdev/struct.LineEvent.html#method.timestamp
torvalds/linux@f885020
torvalds/linux@26d060e

On a positive note, I've really liked using this crate over my various hacky uses of the sysfs API I've used in the past, and my current project wouldn't be possible (other than writing my own version, but I learned about the gpio device interface from this crate), as I'm depending on the timestamp without it. So thanks.

As for the delays on supply, I totally feel that, and I wish I had anything spare, but I'm already developing on a borrowed unit myself...

@warthog618
Copy link

warthog618 commented Oct 24, 2022

Hi. I'm the author of the aforementioned GPIO uAPI v2.

On the original point, uAPI v1 is deprecated in that new development should use v2, but v1 will continue to be supported in the kernel for the foreseeable future. But all new features will go on v2.

Wrt hardware support, the gpio-sim module has been added to the kernel recently (5.19) that makes it much easier to mock hardware for testing. The gpio-sim obsoletes the gpio-mockup module that was very difficult to use in practice, and clearly not well known. So GPIO development doesn't require actual hardware. It does require a recent kernel but that isn't a huge issue for a VM. If you need a library to setup and drive the gpio-sim, my gpiosim library provides that.

Wrt the uAPI changes, v2 is essentially v1 with a generalised and simplified object model and with room for future expansion (e,g, we couldn't readily add debounce to v1). Looking at the gpio-cdev API, seamlessly adding support for v2 will be entertaining, if not impossible, as you mirror the v1 uAPI. libgpiod itself is undergoing a complete API change for this reason, in what will be libgpiod v2. Mapping from v2 to v1 is much simpler - v2 being a superset of v1, and that is what my library gpiocdev does. Feel free to use that as a reference, or just to use it for that matter. It provides complete support for both uAPI v2 and v1, and now has support for async.

@fpagliughi
Copy link
Contributor

fpagliughi commented May 6, 2023

Hey all. Sincere apologies for never getting to this last year. The open-source work was going to be done in parallel with a paid job that was providing some of the motivation, but the job kept getting pushed back, and back, and back... until now.

But looking at the current state of affairs, I wonder if the work is worth it? @warthog618 's library covers most of what I wanted to add to this one (v2 and async-io support), and the gpiosim is a nice bonus.

But... I have a fair amount of code in production that already uses this library, and would need to see what it would take to validate and then switch it all over.

What do other folks think? Has anyone compared the libraries? Is it worth keeping this one going? Or should we mark it obsolete and recommend the newer library for v2 and beyond?

@alexferro
Copy link

I have no real preference here. I am very thankful for the work that has been done on this library, but if the best thing for the ecosystem is to deprecate this and I fix my code the next time I touch the project of mine to use something newer, then so be it. But I also have a very small use case in a tiny application, so take it with the appropriate sized grain of salt.

bors bot added a commit that referenced this issue May 19, 2023
70: Fix LineEvent timestamp doc r=eldruin a=mpi3d

Fix documentation on `LineEvent::timestamp()`.

According to the Linux documentation, the timestamp is `CLOCK_MONOTONIC` by default: [gpio.h#L286](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/gpio.h#n286).

Please see [#68 comment](#68 (comment)).

Co-authored-by: Matthieu CHARETTE <[email protected]>
bors bot added a commit that referenced this issue May 19, 2023
70: Fix LineEvent timestamp doc r=eldruin a=mpi3d

Fix documentation on `LineEvent::timestamp()`.

According to the Linux documentation, the timestamp is `CLOCK_MONOTONIC` by default: [gpio.h#L286](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/gpio.h#n286).

Please see [#68 comment](#68 (comment)).

Co-authored-by: Matthieu CHARETTE <[email protected]>
bors bot added a commit that referenced this issue Jun 5, 2023
70: Fix LineEvent timestamp doc r=eldruin a=mpi3d

Fix documentation on `LineEvent::timestamp()`.

According to the Linux documentation, the timestamp is `CLOCK_MONOTONIC` by default: [gpio.h#L286](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/gpio.h#n286).

Please see [#68 comment](#68 (comment)).

Co-authored-by: Matthieu CHARETTE <[email protected]>
@posborne
Copy link
Member

posborne commented Oct 6, 2023

I plan to revisit this, read up more on uapi v2, and consider what a reasonable plan is moving forward. Right now, I expect that hard requiring uapi v2 would do a disservice to many that may be using systems in production (myself included) that are still using supported kernels that predate the V2 API introduction which, I believe, landed in 5.10. For LTS kernels, that currently includes 4.9 (well, fairly recently expired), 4.14, 4.19, and 5.4 that AFAIK will still receive updates until 2025. I'm actively working to move a few boards to 5.15 from a 4.9 base.

Including the support in this lib seems like it may be preferred with an appropriate API surface and documentation, though I also don't have a really strong objection to pointing people toward gpiocdev. I need to consider a bit more how v1/v2 is handled there to see if it strikes the right balance. In a lot of cases, I think it may be that libs may not know what they will target but may want to be able to determine at runtime to not include some functionality, etc. if V2 is supported.

@warthog618
Copy link

It isn't clear to me what you mean by "striking the right balance" in this context.

One of my goals with gpiocdev was to allow seamless migration from v1 to v2, for all the use cases I could think of, so it:

  • provides a unified API wrapping uAPI v1 and v2
  • allows the user to select supported uAPI version at compile time via features (and you can support both)
  • explicitly supports automatic selection of uAPI version at runtime when built with both uAPI versions
  • allows selection of version at runtime - even on an individual command basis.
  • returns errors at runtime to indicate "this kernel can't do that" (in more detail than that, but that is the gist) if you try to use a v2 specific feature on a v1 kernel
  • provides functions to explicitly check the available version so you can know up front which version you are dealing with

The user is given both reasonable default behaviour and the option to ignore that and do their own thing, so any balancing or trade-off is left to them. But if there is some use case that I'm overlooking then let me know and I'll take a look.

@posborne
Copy link
Member

posborne commented Oct 7, 2023

@warthog618 Excellent, I did a bit more reading and it does seem like you did try to find the right way to "strike the right balance", I just hadn't had a chance to dig into the details when I wrote my previous message and was just going through the thought process of how I think it might be best to approach things and it sounds like you have done similarly with your crate which is great to hear and see.

@fpagliughi
Copy link
Contributor

+1 on @warthog618 's approach. Very usable across versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants