using libgpiod to replace sysfs ABI (was Re: [PATCH] gpiolib: Avoid side effects in gpio_is_visible())

From: Chris Packham
Date: Wed May 24 2023 - 19:53:20 EST

(culled the Cc list but hopefully those that might want to chime in are
on linux-gpio)

On 24/05/23 17:41, Kent Gibson wrote:
> On Tue, May 23, 2023 at 09:17:26PM +0000, Chris Packham wrote:
>> On 24/05/23 04:38, andy.shevchenko@xxxxxxxxx wrote:
>>> Wed, May 17, 2023 at 09:30:51PM +0000, Chris Packham kirjoitti:
>>>> On 17/05/23 20:54, Andy Shevchenko wrote:
>>>>> On Wed, May 17, 2023 at 2:50 AM Chris Packham
>>>>> <Chris.Packham@xxxxxxxxxxxxxxxxxxx> wrote:
>>>>>> On 17/05/23 10:47, Kent Gibson wrote:
>>> ...
>>>> Again the problem boils down to the fact that we have a userspace switch
>>>> driver (which uses a vendor supplied non-free SDK). So despite the
>>>> kernel having quite good support for SFPs I can't use it without a
>>>> netdev to attach it to.
>>> That user space driver is using what from the kernel? GPIO sysfs?
>> Yes GPIO sysfs and exported links with known names, which allows things
>> to be done per-port but also wildcarded from shell scripts if necessary.
>> I think the key point here is that it doesn't care about the GPIO chips
>> just the individual GPIO lines. Anything involving libgpiod currently
>> has to start caring about GPIO chips (or I'm misreading the docs).
> As previously mentioned, the libgpiod tools now support identification of
> lines by name.

The libgpiod tools do but not libgpiod itself. The tools are reasonable
replacements for things that are currently done in shell scripts but
there is also application code that needs to care about GPIO lines but
ideally it shouldn't need to care about GPIO chips.

> As long as your line names are unique at system scope you should be
> good. Otherwise you have no option but to identify by (chip,offset).
> Wrt the library itself, I was thinking about relocating the line name
> resolution logic from the tools into the library itself, so it would be
> more generally accessible, but haven't gotten there yet.

Yes I think that'd help my use-case. Even if there were APIs to iterate
over all possible GPIO lines and let the application worry about how to
match the names.

> I'm also of the opinion that libgpiod is too low level for common
> tasks. That is necessary to access all the features of the uAPI, but
> for basic tasks it would be nice to have a higher level abstraction to
> reduce the barrier to entry.
> e.g. in Rust I can do this:
> let led0 = gpiocdev::find_named_line("LED0").unwrap();
> let req = Request::builder()
> .with_found_line(&led0)
> .as_output(Value::Active)
> .request()?;
> // change value later
> req.set_value(led0.offset, Value::Inactive)
> which is the equivalent of the sysfs
> echo 1 > /some/sysfs/line
> ...
> echo 0 > /some/sysfs/line
> That is bad enough. It pains me to see how complex the equivalent is using
> the libgpiod v2 API (or v1), and that is not putting any shade on Bart or
> anyone else who worked on it - there are a lot of constraints on how it
> is designed. It just doesn't feel complete yet, particularly from a
> casual user's perspective.
> One of the things I would like to see added to libgpiod is a set of working
> examples of simple use cases. Formerly the tools took double duty to
> fill that role, but they've now grown too complex.
> Those examples would highlight where we could provide simplified
> higher level APIs.
> Then rinse and repeat until the simple use cases are simple.

I was a little put-off when I noticed there was an looming API change
the last time I looked at libgpiod and unfortunately any time I had to
spend on updating the application code has now passed.

I think modulo the problem of line discovery the current API would do
what I need. As you've said having some examples in the docs would go a
long way.

It'd also be great if there was some way of ensuring that a line's state
is kept after the application has released the request (i.e. the txdis
case I mentioned). But that probably needs work on the kernel side to
make such guarantees.