Add optional libgpiod support to linuxgpio #1299
Conversation
|
Oops, I left some debug prints in there, let me clean that up real quick... |
SebKuzminsky
force-pushed
the
libgpiod
branch
from
dfe3d4d
to
6d95b15
Compare
|
Ok, now it's ready for review. |
|
This is wonderful. I will carry out some tests soon on my Raspberry Pi 400. |
|
First issue is how to specify the port? git main works fine. |
|
Hmm, thanks for testing, I appreciate the extra eyeballs on this. I'll check it out and get back to you. |
|
This PR currently uses The PR constructs this name by appending the pin number to the string "GPIO", like this: https://github.com/SebKuzminsky/avrdude/blob/6d95b150352622edb1046ce2dc2e2d385a0c5bfd/src/linuxgpio.c#L385-L391. In your example you asked for gpio line 11, which is not named "GPIO11", it's named "SPI_SCLK", so it's not found. In my testing i limited my gpio usage to the ones named "GPIO-something". One effect of this technique that I like is that But I see now the shortcoming that you pointed out - only pins named "GPIO-something" can be used, and there are many pins with other kinds of names. I'll update the PR to accept a gpiochip as the port and the line number (not name) as the gpio identifier on that gpiochip. |
|
See if this works better for you, @mcuee. |
|
Great. Now it works nicely. Tested under Raspberry Pi 400 running 32bit Raspberry Pi OS. |
|
Please take a look at this PR to see if it works for you or not when you have the time. Thanks. |
|
Based on some of the issues reported around libgpiod, I'm happy to see that if I hit Ctrl-C while avrdude is running, it frees the gpio lines (though it does not restore the outputs to inputs). |
|
@sebastiankuzminsky Thanks for contributing! I have had a quick glance over the code and it looks competently written. I don't know much about either of the two interfaces, nor their relative merits. I take it that the gpio sysfs interface is being deprecated, which makes the port to libgpiod desirable. The PR makes libgpiod available when the library exists and works and in this case that supersedes sysfs. @sebastiankuzminsky What is the rationale to keep sysfs? If that actually has been deprecated on a system, could that then mean that avrdude won't compile/work (haven't looked into whether anything else is used over and above file system calls for gpio through sysfs). Might a user still want to work with the old interface? In which case might it be better to have an independent route to using libgpiod? I don't have an opinion on either, but just wanted to make sure these questions have been asked. Planning to do a mergefest in the next few days, so would be good if @mcuee and/or @MCUdude could have a final look before including into the list of PRs to merge. |
|
Hi @stefanrueger, thanks for the comments.
The argument for leaving sysfs support in is so that avrdude will work on old kernels that don't have the newer gpiod interface. avrdude does not need anything beyond normal Posix filesystem stuff to use the old sysfs gpio, i.e. no special build dependencies on external libraries. There is only the runtime dependency of an older kernel that supports that interface. So it doesn't "cost" anything (beyond slightly more complicated code in I did consider leaving "linuxgpio" unchanged as the old sysfs gpio interface, and adding a new driver (maybe confusingly called "linuxgpiod"?) for the new libgpiod interface, but I made the judgement call that having one driver support both of the two linux-kernel gpio interfaces was cleaner. I am of course willing to be overridden on this: if the avrdude developer community prefers, I will change this PR to leave the old sysfs driver alone and add a new gpiod driver next to it. Just let me know (and if so, please suggest a non-confusing name for the new driver). |
|
I agree with @SebKuzminsky to keep both and use libgpiod when it is available and fall back to sysfs when it is available. There are probably quite some Linux machines out there using older kernels than 4.8. Ref: https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/README Since linux 4.8 the GPIO sysfs interface is deprecated. User space should use |
I can't think of a reason, but I am not super well versed in the details of either interface. If the ability to force the linuxgpio driver to use the sysfs interface turns out to be desirable, perhaps we'd use an environment variable, like |
I do not see a reason myself.
I am okay with merging this PR. But I'd like to hear from @MCUdude's input to see if this is already good enough to address the concerns he raised in #932. |
|
Thanks for the PR! I'll see if I can find some time next week to try this out |
|
I have an old 1st gen Raspberry Pi model B that's running the latest Raspbian Bullseye. It's slow as heck, but once it's finished building it works alright. The first issue was that libgpiod wasn't installed by default, and running Programming works like a charm, and IIRC it's also faster than using sysfs. It's also great to see that the GPIOs are freed even if you hit Ctrl+C in the middle of the session. It would be great if there was a default port number so that we didn't have to specify |
|
Hi @MCUdude, thanks for looking. And thanks for maintaining avrdude, it's super useful :-)
I like that idea. I think gpiochip devices always show up as libgpiod has an API to iterate over the gpiochips available (see e.g. the |
|
My thinking before was confused - I was imagining that the programmer's As I understand it, your suggestion to add a Am I understanding your suggestion correctly? |
Yes, that would be the best option IMO. I'm sure most users that use |
|
Ok, here's a proposal for this new feature. I'm not sure what the best way to document that the linuxgpio "port" is only used when linuxgpio uses the gpiod backend, and is ignored when linuxgpio uses the sysfs backend. I'm open to suggestions there. It also occurred to me that there's a user-visible difference in behavior between sysfs and gpiod... The old linuxgpio sysfs backend uses system-wide GPIO pin identifiers, but the new libgpiod backend uses only gpiochip-wide pin identifiers. On the Raspberry Pi for example, there are two "gpiochip" devices ("gpiochip0" and "gpiochip1"), with some pins on each. With sysfs you could potentially use pins from multiple gpiochips in your linuxgpio programmer, but because only one port/gpiochip is available in avrdude at a time, your programmer must fit on one port/gpiochip. On Raspberry Pi, the pins on the main GPIO connector are all on one gpiochip so it works fine, but there might be other platforms that arrange their pins less conveniently. I can imagine something ugly, like "pin numbers are interpreted as |
|
Oh, and i see there are conflicts now between this branch and main, i'm happy to rebase this whenever you think it's time, just let me know. |
|
Thank you for your work! I just tested your last commit, and here are some thoughts:
|
Good point! I just picked a random pin on my Raspberry Pi 4B with the 40-pin header. I moved it to GPIO 22 so it works with the older 26-pin header too.
The commented-out
I updated |
|
Where are we with this PR? Good to merge?
@sebastiankuzminsky Please do! |
…sfs" This commit renames the internal functions and variables of the linuxgpio pin driver to include "sysfs", to indicate that they're referring to the older sysfs interface. The following commits will add libgpiod support to the linuxgpio pin driver, and autoselect between them.
|
Rebased :-) |
|
Thanks! This looks like an excellent contribution. Earmarked for next mergefest (unless I hear from the other maintainers). Tiny point though: we tend to leave the programming/white space style from previous contributors unchanged. The many white space changes in And thank you for the contribution in the first place. |
Oops, I goofed that up during the rebase, sorry. Re-rebase coming up! |
The "linuxgpio" backend will now try both the old sysfs and new (since Linux 4.8) libgpiod interface to access gpio pins. If libgpiod works it will use that, if it does not work it will fall back to the old sysfs interface. The libgpiod code is compiled in if libgpiod is available at build time, and compiled out otherwise.
Before this commit, linuxgpio-gpiod used `gpiod_line_find()` to find gpio lines by name, and would construct the gpio name by appending the requested gpio number to the string "GPIO". This had the advantage that it didn't have to specify the gpiochip to use, since gpio names are globally unique in gpiod-land. But it has the severe drawback that only gpios whose name begins with "GPIO" could be used. This commit changes that to use `gpiod_line_get()` instead, which takes a gpiochip name (the avrdude "port") and an "offset" (the pin number on that chip). This lets it use any available gpio line, no matter what name libgpiod assigned it.
This also adds a default linuxgpio port ("gpiochip0"), and adds a
programmer to the avrdude.conf file using linuxgpio from the Raspberry
Pi GPIO port.
This makes the raspberry_pi_gpio programmer work on older Pis with the smaller 26-pin GPIO header. Updates based on code review by Hans (MCUdude).
|
Ok, fixed I think. Thanks @stefanrueger for catching that. |
|
Hey @stefanrueger @SebKuzminsky I am using a Raspberry Pi 4B (40 pin header) This is the error I am gettingIt's the same for -P GPIO Snippet of my avrdude.confAnd the content of /sys/class/gpio/Do you know, what I am doing wrong? |
Thanks for reporting. Naive question: is GPIO 25 already exported/busy? Other than that, perhaps best to raise an issue rather than commenting in a closed PR. This to increase visibility of the problem you experience. |
This PR updates the linuxgpio backend to try both the old sysfs interface and the new (since Linux 4.8, so not that new) libgpiod interface.
I tested this on a Raspberry Pi 4 (running Debian Bookworm arm64), flashing a little attiny2313. I used the same config file that works on an older system with the sysfs interface.
Fixes #831.