Re: Assign line names at runtime

Re: Assign line names at runtime

[Westermann, Oliver]

Hey and thanks for your responses, those are actually quite insightful.

What I read from that is that changing line names really has a lot of implications.

Kent Gibson wrote:
> Alternatively, are named lines the right solution to your problem?
> Is it important to you that the lines are correctly named, or are you
> just using the name for the chip/offset lookup? 

We would really like to use named lines as they are really convinient, but your question actually made me rethink my initial question. We do actually not want to change line names, they are constant throughout the runtime of a device.

> If the latter perhaps roll your own pinout lookup based on the platform configuration?

The truth might lay in between: We would prefer to use existing features and standard interfaces instead of rolling out our own layer. But maybe it's just the initial naming that we want to move. A better solution might be to add another option to define and probe the GPIO driver at runtime: Instead of being required to set all information in the dtb (and therefore from a very low level), we might trigger the probing through modprobe and provide the GPIO line names from userspace. I'm not sure if such an option exists currently?

Best regards and sorry for the quoting style, our mailservers mess with your mails.

- Olli

Re: Assign line names at runtime

[Kent Gibson]

On Thu, Jan 11, 2024 at 04:52:24PM +0000, Westermann, Oliver wrote:
> Hey and thanks for your responses, those are actually quite insightful.
>
> What I read from that is that changing line names really has a lot of
> implications.
>

After sleeping on it, I don't think line renaming is actually such a big issue.

Firstly, hot plugging means the line namespace is never going to be
static, even if I was tacitly assuming it was.  Turns out I don't think
that matters as much as I thought anyway.  We just need to make sure the
user is aware of it as well.

The analogy I would use is files in a filesystem, where the chip
corresponds to a directory and the line a file.  We aren't terribly
concerned that a file may be renamed while we do a find, or while
opening or using a file, so it should be the same for a line.

We rename a file through the directory, so it makes sense to rename the
line through the chip, and not require the line to be requested.
So we would add a new ioctl on the chip to perform the rename.
Could make that more general in case we ever add something else to line
info that isn't controlled by requesting the line, but I'm note sure the
additional complexity would be worth it, given how unlikely that is.
But I digress...

We don't inform a user with an open file that it may have been renamed
while open, so neither would we with the line. If it is an issue for you
then you can add a watch on the line info, similar to using inotify
on the filesystem.

The point the analogy breaks down a bit is that we allow duplicate names,
(I don't think anyone really wants that feature, but historically it has
been allowed so we're stuck with it.) but I don't think that is of any
consequence to this discussion.

If we did want to provide a consistent view of the line namespace, that
might be something the GPIO daemon provides. (conveniently handballing
the problem to Bart ;-)

> Kent Gibson wrote:
> > Alternatively, are named lines the right solution to your problem?
> > Is it important to you that the lines are correctly named, or are you
> > just using the name for the chip/offset lookup?
>
> We would really like to use named lines as they are really convinient, but
> your question actually made me rethink my initial question. We do actually
> not want to change line names, they are constant throughout the runtime of
> a device.
>
> > If the latter perhaps roll your own pinout lookup based on the platform configuration?
>
> The truth might lay in between: We would prefer to use existing features and
> standard interfaces instead of rolling out our own layer. But maybe it's
> just the initial naming that we want to move. A better solution might be to
> add another option to define and probe the GPIO driver at runtime: Instead
> of being required to set all information in the dtb (and therefore from a
> very low level), we might trigger the probing through modprobe and provide
> the GPIO line names from userspace. I'm not sure if such an option exists
> currently?
>

That sounds like the job of the udev rules that Bart suggested - once we have
the ability to rename lines from userspace.

> Best regards and sorry for the quoting style, our mailservers mess with your
> mails.
>

No problem with your quoting style - that looks fine to me - it was the lack
of line wrapping that was the issue. And your response isn't tied into
the email thread either, which is a bit unfortunate.

Oh, and it would be handy to prefix libgpiod specific questions with
[libgpiod], though in this case it has rapidly moved into kernel space
anyway, so no biggy - just for future reference.

Cheers,
Kent.

Re: Assign line names at runtime

[Bartosz Golaszewski]

On Fri, Jan 12, 2024 at 1:36 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Thu, Jan 11, 2024 at 04:52:24PM +0000, Westermann, Oliver wrote:
> > Hey and thanks for your responses, those are actually quite insightful.
> >
> > What I read from that is that changing line names really has a lot of
> > implications.
> >
>
> After sleeping on it, I don't think line renaming is actually such a big issue.
>
> Firstly, hot plugging means the line namespace is never going to be
> static, even if I was tacitly assuming it was.  Turns out I don't think
> that matters as much as I thought anyway.  We just need to make sure the
> user is aware of it as well.
>
> The analogy I would use is files in a filesystem, where the chip
> corresponds to a directory and the line a file.  We aren't terribly
> concerned that a file may be renamed while we do a find, or while
> opening or using a file, so it should be the same for a line.
>
> We rename a file through the directory, so it makes sense to rename the
> line through the chip, and not require the line to be requested.
> So we would add a new ioctl on the chip to perform the rename.
> Could make that more general in case we ever add something else to line
> info that isn't controlled by requesting the line, but I'm note sure the
> additional complexity would be worth it, given how unlikely that is.
> But I digress...
>
> We don't inform a user with an open file that it may have been renamed
> while open, so neither would we with the line. If it is an issue for you
> then you can add a watch on the line info, similar to using inotify
> on the filesystem.
>
> The point the analogy breaks down a bit is that we allow duplicate names,
> (I don't think anyone really wants that feature, but historically it has
> been allowed so we're stuck with it.) but I don't think that is of any
> consequence to this discussion.
>

This analogy is great and all but there's one issue with it - we're
not dealing with a filesystem that everyone can modify. We have more
or less agreed so far that we allow multiple readers of GPIO
information but whenever there's any setting done, one needs to
request the line for exclusive usage. Now we'd break that logic by
allowing all users to arbitrarily rename GPIO lines and I don't like
this.

> If we did want to provide a consistent view of the line namespace, that
> might be something the GPIO daemon provides. (conveniently handballing
> the problem to Bart ;-)
>
> > Kent Gibson wrote:
> > > Alternatively, are named lines the right solution to your problem?
> > > Is it important to you that the lines are correctly named, or are you
> > > just using the name for the chip/offset lookup?
> >
> > We would really like to use named lines as they are really convinient, but
> > your question actually made me rethink my initial question. We do actually
> > not want to change line names, they are constant throughout the runtime of
> > a device.
> >
> > > If the latter perhaps roll your own pinout lookup based on the platform configuration?
> >
> > The truth might lay in between: We would prefer to use existing features and
> > standard interfaces instead of rolling out our own layer. But maybe it's
> > just the initial naming that we want to move. A better solution might be to
> > add another option to define and probe the GPIO driver at runtime: Instead
> > of being required to set all information in the dtb (and therefore from a
> > very low level), we might trigger the probing through modprobe and provide
> > the GPIO line names from userspace. I'm not sure if such an option exists
> > currently?
> >
>
> That sounds like the job of the udev rules that Bart suggested - once we have
> the ability to rename lines from userspace.
>

It would make sense if we could get a udev event about the device
being created, pass device properties - in this case gpio-line-names -
to the kernel for this device and then get it bound to the driver. I
don't think it's possible but I need to look deeper.

Bartosz

> > Best regards and sorry for the quoting style, our mailservers mess with your
> > mails.
> >
>
> No problem with your quoting style - that looks fine to me - it was the lack
> of line wrapping that was the issue. And your response isn't tied into
> the email thread either, which is a bit unfortunate.
>
> Oh, and it would be handy to prefix libgpiod specific questions with
> [libgpiod], though in this case it has rapidly moved into kernel space
> anyway, so no biggy - just for future reference.
>
> Cheers,
> Kent.

Re: Assign line names at runtime

[Kent Gibson]

On Fri, Jan 12, 2024 at 12:26:55PM +0100, Bartosz Golaszewski wrote:
> On Fri, Jan 12, 2024 at 1:36 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Thu, Jan 11, 2024 at 04:52:24PM +0000, Westermann, Oliver wrote:
> > > Hey and thanks for your responses, those are actually quite insightful.
> > >
> > > What I read from that is that changing line names really has a lot of
> > > implications.
> > >
> >
> > After sleeping on it, I don't think line renaming is actually such a big issue.
> >
> > Firstly, hot plugging means the line namespace is never going to be
> > static, even if I was tacitly assuming it was.  Turns out I don't think
> > that matters as much as I thought anyway.  We just need to make sure the
> > user is aware of it as well.
> >
> > The analogy I would use is files in a filesystem, where the chip
> > corresponds to a directory and the line a file.  We aren't terribly
> > concerned that a file may be renamed while we do a find, or while
> > opening or using a file, so it should be the same for a line.
> >
> > We rename a file through the directory, so it makes sense to rename the
> > line through the chip, and not require the line to be requested.
> > So we would add a new ioctl on the chip to perform the rename.
> > Could make that more general in case we ever add something else to line
> > info that isn't controlled by requesting the line, but I'm note sure the
> > additional complexity would be worth it, given how unlikely that is.
> > But I digress...
> >
> > We don't inform a user with an open file that it may have been renamed
> > while open, so neither would we with the line. If it is an issue for you
> > then you can add a watch on the line info, similar to using inotify
> > on the filesystem.
> >
> > The point the analogy breaks down a bit is that we allow duplicate names,
> > (I don't think anyone really wants that feature, but historically it has
> > been allowed so we're stuck with it.) but I don't think that is of any
> > consequence to this discussion.
> >
>
> This analogy is great and all but there's one issue with it - we're
> not dealing with a filesystem that everyone can modify. We have more
> or less agreed so far that we allow multiple readers of GPIO
> information but whenever there's any setting done, one needs to
> request the line for exclusive usage. Now we'd break that logic by
> allowing all users to arbitrarily rename GPIO lines and I don't like
> this.
>

Firstly, sorry if I gave the impression I'm all in on this idea - this
is still spitballing.

You are extending the analogy too far, I'm only applying it to names and how
name instability might be viewed from the user's perspective.
I was thinking it could be a deal-breaker, but if it is ok for the
filesystem then maybe not.

I have no problem considering the line name to be metadata, not data (line
configuration and value).

You aren't allowing all users - you are allowing those that have
permission to open the chip, so it can be locked down if necessary, the same
as requesting the line.

Having said all that, I am uneasy that this capability could be abused,
particularly in ways we can't anticipate.
So I'm at the point that I think we could do it, one way or another, but
much less certain if we should.
I would not consider it if there was an alternative.

Cheers,
Kent.

Re: Re: Assign line names at runtime

[Westermann, Oliver]

You mean you specify those in .dts?

On Fri, Jan 12, 2024 at 9:33 Alexander Dahl wrote:
> Am Thu, Jan 11, 2024 at 10:42:51AM +0000 schrieb Westermann, Oliver:
> > We're transitioning from using the old sysfs interface to using
> > gpiod and named lines. For most devices, we specify line names at
> > boot time using gpio-line-names.
> 
> You mean you specify those in .dts?

Yes, currently we define the full GPIO expanders in .dts, bus, address, gpio-line-names.

> > On some devices we have small differences between revisions or
> > hardware variants, which causes lines to be swapped on GPIO
> > expanders or just being used differently for between revisions. We
> > started to handle this by overlays, but that requires to distinguish
> > during the bootloader phase, which is hard to service and often
> > unneeded. Especially when we want to rename a single line, the
> > overlay needs to override all entries, which leads to duplication of
> > those line name lists.
> 
> So essentially you have hardware variants.  In my opinion this should
> be handled in the bootloader.  What about having a .dtsi for the
> common part of the board, one .dts file for each variant, and the
> bootloader picking the correct one?  This is probably less complicated
> than handling with overlays.  Overlays are designated for a different
> use case like add-on boards, aren't they?

In general, I'm focusing on the add-on boards. For the baseboards, we can work
with .dtb(i/o), as the mutations are limited and changes are rare, so those can
be handled using in the bootloader. But our add-on boards are plentyful and
have variants with revisions each. Supporting them all in bootloader/dtb means
regular updates to low-level code, which feels like we can simplify and derisk
it.

On Fri, Jan 12, 2024 at 1:31 AM Kent Gibson wrote:
> So I'm at the point that I think we could do it, one way or another, but
> much less certain if we should.
> I would not consider it if there was an alternative.

I played around a bit this morning and I have a (probably hacky but) working
prototype which adds a GPIO_V2_SET_LINEINFO_IOCTL and currently only allows to
override a line name. I played around a bit and tried to break something, but
currently, it seems to work. But I'm also open for any alternative, so maybe
with some extra info, somebody has better ideas:

The hardware variants I'm dealing with could be considered accessories:
Extension of a base in different kind and revisions. As those, they are mostly
not boot critical - I can defer probing. That would also allow me to defer
probing up until manual probing from userspace, eg by a udev rule collecting
more data and providing that to the driver once all data is present.

Could e.g. an extension of the modprobe params for i2c gpiochip drivers allow to
provide not only bus and address, but also a list of pin names? Ideally
implemented on the gpiochip / i2c gpio level so this applies to all gpio drivers?

(new attempt at manual formatting, sorry)

Best regards, Olli

Re: Re: Assign line names at runtime

[Kent Gibson]

On Fri, Jan 12, 2024 at 01:16:29PM +0000, Westermann, Oliver wrote:
>
>
> On Fri, Jan 12, 2024 at 1:31 AM Kent Gibson wrote:
> > So I'm at the point that I think we could do it, one way or another, but
> > much less certain if we should.
> > I would not consider it if there was an alternative.
>
> I played around a bit this morning and I have a (probably hacky but) working
> prototype which adds a GPIO_V2_SET_LINEINFO_IOCTL and currently only allows to
> override a line name. I played around a bit and tried to break something, but
> currently, it seems to work. But I'm also open for any alternative, so maybe
> with some extra info, somebody has better ideas:
>
> The hardware variants I'm dealing with could be considered accessories:
> Extension of a base in different kind and revisions. As those, they are mostly
> not boot critical - I can defer probing. That would also allow me to defer
> probing up until manual probing from userspace, eg by a udev rule collecting
> more data and providing that to the driver once all data is present.
>
> Could e.g. an extension of the modprobe params for i2c gpiochip drivers allow to
> provide not only bus and address, but also a list of pin names? Ideally
> implemented on the gpiochip / i2c gpio level so this applies to all gpio drivers?
>
> (new attempt at manual formatting, sorry)
>

Sorry for the belated reply, but just to clarify where I am with this:

This looks like an init-time problem, more so than a runtime, so you
should pursue the init-time solutions and exhaust your options there
before looking to solve it via the GPIO uAPI.

Cheers,
Kent.