Following our discussion[1], here's a proposal for extending the sysfs
interface with attributes not referring to GPIO lines by their global
numbers in a backward compatible way.

Long story short: there is now a new class device for each GPIO chip.
It's called chipX where X is the ID of the device as per the driver
model and it lives next to the old gpiochipABC where ABC is the GPIO
base. Each new chip class device has a pair of export/unexport
attributes which work similarly to the global ones under /sys/class/gpio
but take hardware offsets within the chip as input, instead of the
global numbers. Finally, each exported line appears at the same time as
the global /sys/class/gpio/gpioABC as well as per-chip
/sys/class/gpio/chipX/gpioY sysfs group.

The series contains the implementation of a parallel GPIO chip entry not
containing the base GPIO number in the name and the corresponding sysfs
attribute group for each exported line that lives under the new chip
class device as well as a way to allow to compile out the legacy parts
leaving only the new elements of the sysfs ABI.

This series passes the compatibility tests I wrote while working on the
user-space compatibility layer for sysfs[2].

[1] https://lore.kernel.org/all/CAMRc=McUCeZcU6co1aN54rTudo+JfPjjForu4iKQ5npwXk6GXA@mail.gmail.com/
[2] https://github.com/brgl/gpio-sysfs-compat-tests

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Changes in v3:
- wrap a local variable in #ifdef's as it's only used if legacy sysfs
  ABI is enabled (Geert)
- fix an issue with jumping over a variable annotated with __free()
- use gpiod_is_equal() where applicable
- fix a use-after-free issue
- remove trailing commas from NULL array terminators
- improve commit messages
- change patch order for smaller diffstats and better readability
- put struct list_head at the beginning of structures for better
  performance
- reshuffle the contents of struct gpiodev_data: put all legacy fields
  at the end
- don't break lines too eagerly
- Link to v2: https://lore.kernel.org/r/20250623-gpio-sysfs-chip-export-v2-0-d592793f8964@linaro.org

Changes in v2:
- add missing call to sysfs_remove_groups() in gpiod_unexport()
- pick up review tags
- drop patches that were already applied
- add missing documentation for the chip-local line attributes
- correct the statement about chip's label uniqueness in docs
- Link to v1: https://lore.kernel.org/r/20250610-gpio-sysfs-chip-export-v1-0-a8c7aa4478b1@linaro.org

---
Bartosz Golaszewski (10):
      gpio: sysfs: use gpiod_is_equal() to compare GPIO descriptors
      gpio: sysfs: add a parallel class device for each GPIO chip using device IDs
      gpio: sysfs: only get the dirent reference for the value attr once
      gpio: sysfs: pass gpiod_data directly to internal GPIO sysfs functions
      gpio: sysfs: rename the data variable in gpiod_(un)export()
      gpio: sysfs: don't use driver data in sysfs callbacks for line attributes
      gpio: sysfs: don't look up exported lines as class devices
      gpio: sysfs: export the GPIO directory locally in the gpiochip<id> directory
      gpio: sysfs: allow disabling the legacy parts of the GPIO sysfs interface
      gpio: TODO: remove the task for the sysfs rework

 Documentation/ABI/obsolete/sysfs-gpio |  12 +-
 drivers/gpio/Kconfig                  |   8 +
 drivers/gpio/TODO                     |  13 -
 drivers/gpio/gpiolib-sysfs.c          | 504 +++++++++++++++++++++++++---------
 4 files changed, 389 insertions(+), 148 deletions(-)
---
base-commit: 26981e8906bb5c902e2d34874f64ecfa975d28c8
change-id: 20250402-gpio-sysfs-chip-export-84ac424b61c5

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

On Mon, Jun 30, 2025 at 02:37:07PM +0200, Bartosz Golaszewski wrote:
> Following our discussion[1], here's a proposal for extending the sysfs
> interface with attributes not referring to GPIO lines by their global
> numbers in a backward compatible way.
> 
> Long story short: there is now a new class device for each GPIO chip.
> It's called chipX where X is the ID of the device as per the driver
> model and it lives next to the old gpiochipABC where ABC is the GPIO
> base. Each new chip class device has a pair of export/unexport
> attributes which work similarly to the global ones under /sys/class/gpio
> but take hardware offsets within the chip as input, instead of the
> global numbers. Finally, each exported line appears at the same time as
> the global /sys/class/gpio/gpioABC as well as per-chip
> /sys/class/gpio/chipX/gpioY sysfs group.
> 
> The series contains the implementation of a parallel GPIO chip entry not
> containing the base GPIO number in the name and the corresponding sysfs
> attribute group for each exported line that lives under the new chip
> class device as well as a way to allow to compile out the legacy parts
> leaving only the new elements of the sysfs ABI.
> 
> This series passes the compatibility tests I wrote while working on the
> user-space compatibility layer for sysfs[2].

It seems I never expressed my overall opinion about this. I think the poking
sysfs and making it working with a new schema won't solve the issues that
character device was developed to target. If so, doing this just brings yet
another broken interface. I would be happy to be mistaken!

If I am mistaken, I would like to see a summary here that explains that clearly
that the new sysfs approach does not inherit design flaws of the original
implementation.

> [1] https://lore.kernel.org/all/CAMRc=McUCeZcU6co1aN54rTudo+JfPjjForu4iKQ5npwXk6GXA@mail.gmail.com/
> [2] https://github.com/brgl/gpio-sysfs-compat-tests

-- 
With Best Regards,
Andy Shevchenko

On Tue, Jul 01, 2025 at 05:05:10PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 30, 2025 at 02:37:07PM +0200, Bartosz Golaszewski wrote:
> > Following our discussion[1], here's a proposal for extending the sysfs
> > interface with attributes not referring to GPIO lines by their global
> > numbers in a backward compatible way.
> >
> > Long story short: there is now a new class device for each GPIO chip.
> > It's called chipX where X is the ID of the device as per the driver
> > model and it lives next to the old gpiochipABC where ABC is the GPIO
> > base. Each new chip class device has a pair of export/unexport
> > attributes which work similarly to the global ones under /sys/class/gpio
> > but take hardware offsets within the chip as input, instead of the
> > global numbers. Finally, each exported line appears at the same time as
> > the global /sys/class/gpio/gpioABC as well as per-chip
> > /sys/class/gpio/chipX/gpioY sysfs group.
> >
> > The series contains the implementation of a parallel GPIO chip entry not
> > containing the base GPIO number in the name and the corresponding sysfs
> > attribute group for each exported line that lives under the new chip
> > class device as well as a way to allow to compile out the legacy parts
> > leaving only the new elements of the sysfs ABI.
> >
> > This series passes the compatibility tests I wrote while working on the
> > user-space compatibility layer for sysfs[2].
>
> It seems I never expressed my overall opinion about this. I think the poking
> sysfs and making it working with a new schema won't solve the issues that
> character device was developed to target. If so, doing this just brings yet
> another broken interface. I would be happy to be mistaken!
>
> If I am mistaken, I would like to see a summary here that explains that clearly
> that the new sysfs approach does not inherit design flaws of the original
> implementation.
>

Indeed.  I've already expressed my reservations about supporting the whole
of the existing sysfs capabilties, but I've otherwise tried to remain out
of the discussion.

To reiterate my position:
While I am all for maintaining sysfs in some form to cater for those
rare cases where cdev is too heavyweight, IMHO it is a mistake to
support the existing sysfs capabilities in toto.  Take the opportunity to
remove the parts of the sysfs interface that don't work well.
The new sysfs should only provide the features required by those rare use
cases, which IIUC would be basic sets and gets, and exclude those features
not required, particularly warts like edges.

If you need more advanced features then use cdev.
If all you need is basic sets and gets then sysfs is probably fine.

If that isn't the case then there should be some explanation as to why those
sysfs features are being maintained.  Treat this as a new interface.

Cheers,
Kent.

On Wed, Jul 2, 2025 at 5:54 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Tue, Jul 01, 2025 at 05:05:10PM +0300, Andy Shevchenko wrote:
> >
> > It seems I never expressed my overall opinion about this. I think the poking
> > sysfs and making it working with a new schema won't solve the issues that
> > character device was developed to target. If so, doing this just brings yet
> > another broken interface. I would be happy to be mistaken!
> >
> > If I am mistaken, I would like to see a summary here that explains that clearly
> > that the new sysfs approach does not inherit design flaws of the original
> > implementation.
> >

You cut out the link to the discussion that preceded this series where
a good summary is in the very first email. Anyway: the gist is: people
need to do some basic GPIO fiddling early on from initramfs that may
not have any tools other than basic shell utils from busybox. This
series is not about improving or extending the sysfs interface - it's
about removing its reliance on global GPIO numbers. And that's about
it. We don't add any new features really, just move the GPIO line
groups into their respective chip directories and make exporting use
the hardware offsets, not global numbers.

>
> Indeed.  I've already expressed my reservations about supporting the whole
> of the existing sysfs capabilties, but I've otherwise tried to remain out
> of the discussion.
>
> To reiterate my position:
> While I am all for maintaining sysfs in some form to cater for those
> rare cases where cdev is too heavyweight, IMHO it is a mistake to
> support the existing sysfs capabilities in toto.  Take the opportunity to
> remove the parts of the sysfs interface that don't work well.

Doesn't the last patch do it? We cannot remove it without giving
user-space some time to switch. This series does everything in a
backward compatible way and then isolates the old bits under ifdefs so
that when the time comes it's just a matter of removing everything
guarded by them.

> The new sysfs should only provide the features required by those rare use
> cases, which IIUC would be basic sets and gets, and exclude those features
> not required, particularly warts like edges.
>
> If you need more advanced features then use cdev.
> If all you need is basic sets and gets then sysfs is probably fine.
>
> If that isn't the case then there should be some explanation as to why those
> sysfs features are being maintained.  Treat this as a new interface.
>

I tend to not interpret it as adding new features. We really just
*move* what exists under a slightly different path when you think
about it.

So what are you suggesting, remove the `edge` attribute and polling
features from the new `value` attribute?

Bart

On Wed, Jul 02, 2025 at 11:45:02AM +0200, Bartosz Golaszewski wrote:
> On Wed, Jul 2, 2025 at 5:54 AM Kent Gibson <warthog618@gmail.com> wrote:
> > On Tue, Jul 01, 2025 at 05:05:10PM +0300, Andy Shevchenko wrote:
> > > It seems I never expressed my overall opinion about this. I think the poking
> > > sysfs and making it working with a new schema won't solve the issues that
> > > character device was developed to target. If so, doing this just brings yet
> > > another broken interface. I would be happy to be mistaken!
> > >
> > > If I am mistaken, I would like to see a summary here that explains that clearly
> > > that the new sysfs approach does not inherit design flaws of the original
> > > implementation.
> 
> You cut out the link to the discussion that preceded this series where
> a good summary is in the very first email. Anyway: the gist is: people
> need to do some basic GPIO fiddling early on from initramfs that may
> not have any tools other than basic shell utils from busybox. This
> series is not about improving or extending the sysfs interface - it's
> about removing its reliance on global GPIO numbers. And that's about
> it. We don't add any new features really, just move the GPIO line
> groups into their respective chip directories and make exporting use
> the hardware offsets, not global numbers.

I see it differently. This adds the second variant of how sysfs can be handled
and it needs to be rotten in the same way as the original sysfs. I really don't
see a point to prolonging the life of the broken interface in such a way. If somebody
wants to check the GPIO without accessing character device interface, they probably
are simply lazy to think of how to do that on early stages properly. The desire
sounds like a workaround against proper thinking.

> > Indeed.  I've already expressed my reservations about supporting the whole
> > of the existing sysfs capabilties, but I've otherwise tried to remain out
> > of the discussion.
> >
> > To reiterate my position:
> > While I am all for maintaining sysfs in some form to cater for those
> > rare cases where cdev is too heavyweight, IMHO it is a mistake to
> > support the existing sysfs capabilities in toto.  Take the opportunity to
> > remove the parts of the sysfs interface that don't work well.
> 
> Doesn't the last patch do it? We cannot remove it without giving
> user-space some time to switch.

Famous Last Words. How many years the sysfs is being rotten?! This just makes
it a Frankenstein.

> This series does everything in a
> backward compatible way and then isolates the old bits under ifdefs so
> that when the time comes it's just a matter of removing everything
> guarded by them.
> 
> > The new sysfs should only provide the features required by those rare use
> > cases, which IIUC would be basic sets and gets, and exclude those features
> > not required, particularly warts like edges.
> >
> > If you need more advanced features then use cdev.
> > If all you need is basic sets and gets then sysfs is probably fine.
> >
> > If that isn't the case then there should be some explanation as to why those
> > sysfs features are being maintained.  Treat this as a new interface.
> 
> I tend to not interpret it as adding new features. We really just
> *move* what exists under a slightly different path when you think
> about it.
> 
> So what are you suggesting, remove the `edge` attribute and polling
> features from the new `value` attribute?

-- 
With Best Regards,
Andy Shevchenko

On Wed, Jul 2, 2025 at 1:55 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Wed, Jul 02, 2025 at 11:45:02AM +0200, Bartosz Golaszewski wrote:
> > On Wed, Jul 2, 2025 at 5:54 AM Kent Gibson <warthog618@gmail.com> wrote:
> > > On Tue, Jul 01, 2025 at 05:05:10PM +0300, Andy Shevchenko wrote:
> > > > It seems I never expressed my overall opinion about this. I think the poking
> > > > sysfs and making it working with a new schema won't solve the issues that
> > > > character device was developed to target. If so, doing this just brings yet
> > > > another broken interface. I would be happy to be mistaken!
> > > >
> > > > If I am mistaken, I would like to see a summary here that explains that clearly
> > > > that the new sysfs approach does not inherit design flaws of the original
> > > > implementation.
> >
> > You cut out the link to the discussion that preceded this series where
> > a good summary is in the very first email. Anyway: the gist is: people
> > need to do some basic GPIO fiddling early on from initramfs that may
> > not have any tools other than basic shell utils from busybox. This
> > series is not about improving or extending the sysfs interface - it's
> > about removing its reliance on global GPIO numbers. And that's about
> > it. We don't add any new features really, just move the GPIO line
> > groups into their respective chip directories and make exporting use
> > the hardware offsets, not global numbers.
>
> I see it differently. This adds the second variant of how sysfs can be handled
> and it needs to be rotten in the same way as the original sysfs. I really don't
> see a point to prolonging the life of the broken interface in such a way. If somebody
> wants to check the GPIO without accessing character device interface, they probably
> are simply lazy to think of how to do that on early stages properly. The desire
> sounds like a workaround against proper thinking.
>

Whatever your opinion on this is - if user-space wants to keep the
interface, then we need to support it. We can only propose
alternatives and hope the users will switch. Please read the
discussion, it explains why people want to keep using the simple sysfs
ABI and why those specific users will most likely never switch to the
character device. At this point a bigger concern to me is the global
GPIO numberspace, not the existence of the sysfs class as such.

We have three alternatives:

1. Do nothing. Keep the sysfs as is and hope we'll drop it eventually
but this will most likely never happen.
2. Add an entirely new "simple" interface next to the existing sysfs
AND character device. It seems to be what you're proposing but this is
the worst of two worlds. I don't want to be in charge of maintaining
three separate interfaces. Four if you count cdev v1.
3. Modify the existing sysfs in a backward compatible way but make it
possible to export lines by their HW offset within their parent chip,
not by global numbers. This is what this series does and what people
having interest in using sysfs confirmed would work for them.
Eventually we could drop the bits that use the global numberspace
which seems more palatable for user-space than entirely removing the
sysfs GPIO class.

> > > Indeed.  I've already expressed my reservations about supporting the whole
> > > of the existing sysfs capabilties, but I've otherwise tried to remain out
> > > of the discussion.
> > >
> > > To reiterate my position:
> > > While I am all for maintaining sysfs in some form to cater for those
> > > rare cases where cdev is too heavyweight, IMHO it is a mistake to
> > > support the existing sysfs capabilities in toto.  Take the opportunity to
> > > remove the parts of the sysfs interface that don't work well.
> >
> > Doesn't the last patch do it? We cannot remove it without giving
> > user-space some time to switch.
>
> Famous Last Words. How many years the sysfs is being rotten?! This just makes
> it a Frankenstein.
>

I'm doing it BECAUSE sysfs hasn't gone anywhere despite several years
of efforts. I cannot force user-space to stop using it, I can only
propose alternatives and it seems that cdev has not been enough.

Bart

On Wed, Jul 2, 2025 at 3:19 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Whatever your opinion on this is - if user-space wants to keep the
> interface, then we need to support it. We can only propose
> alternatives and hope the users will switch. Please read the
> discussion, it explains why people want to keep using the simple sysfs
> ABI and why those specific users will most likely never switch to the
> character device. At this point a bigger concern to me is the global
> GPIO numberspace, not the existence of the sysfs class as such.

I agree with Bart here, the global GPIO numberspace is the big
problem we need to get rid of, if we move users over to the new
sysfs ABI and disable and finally delete the old one, we can get
rid of the global GPIO numberspace.

This will be needed to conclude the work inside the kernel do
move everyone and their dog over to using GPIO descriptors
instead of numbers.

We would maybe be able to delete <linux/gpio.h> but without
also getting rid of the global numberspace it will feel very
incomplete.

Yours,
Linus Walleij

On Wed, Jul 02, 2025 at 11:45:02AM +0200, Bartosz Golaszewski wrote:
> On Wed, Jul 2, 2025 at 5:54 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Tue, Jul 01, 2025 at 05:05:10PM +0300, Andy Shevchenko wrote:
> > >
> > > It seems I never expressed my overall opinion about this. I think the poking
> > > sysfs and making it working with a new schema won't solve the issues that
> > > character device was developed to target. If so, doing this just brings yet
> > > another broken interface. I would be happy to be mistaken!
> > >
> > > If I am mistaken, I would like to see a summary here that explains that clearly
> > > that the new sysfs approach does not inherit design flaws of the original
> > > implementation.
> > >
>
> You cut out the link to the discussion that preceded this series where
> a good summary is in the very first email. Anyway: the gist is: people
> need to do some basic GPIO fiddling early on from initramfs that may
> not have any tools other than basic shell utils from busybox. This
> series is not about improving or extending the sysfs interface - it's
> about removing its reliance on global GPIO numbers. And that's about
> it. We don't add any new features really, just move the GPIO line
> groups into their respective chip directories and make exporting use
> the hardware offsets, not global numbers.
>

And that is the problem I have with it - it is just removing the global
numbering, while keeping all the other sysfs baggage.
Instead I think it should be thought of as adding a new minimal sysfs
alternative to cdev, based on the old sysfs.

> >
> > Indeed.  I've already expressed my reservations about supporting the whole
> > of the existing sysfs capabilties, but I've otherwise tried to remain out
> > of the discussion.
> >
> > To reiterate my position:
> > While I am all for maintaining sysfs in some form to cater for those
> > rare cases where cdev is too heavyweight, IMHO it is a mistake to
> > support the existing sysfs capabilities in toto.  Take the opportunity to
> > remove the parts of the sysfs interface that don't work well.
>
> Doesn't the last patch do it? We cannot remove it without giving
> user-space some time to switch. This series does everything in a
> backward compatible way and then isolates the old bits under ifdefs so
> that when the time comes it's just a matter of removing everything
> guarded by them.
>

Not suggesting any changes to the existing sysfs here, only your new.

> > The new sysfs should only provide the features required by those rare use
> > cases, which IIUC would be basic sets and gets, and exclude those features
> > not required, particularly warts like edges.
> >
> > If you need more advanced features then use cdev.
> > If all you need is basic sets and gets then sysfs is probably fine.
> >
> > If that isn't the case then there should be some explanation as to why those
> > sysfs features are being maintained.  Treat this as a new interface.
> >
>
> I tend to not interpret it as adding new features. We really just
> *move* what exists under a slightly different path when you think
> about it.
>
> So what are you suggesting, remove the `edge` attribute and polling
> features from the new `value` attribute?
>

Exactly. I'm not suggesting ANY changes to the old sysfs, only your new
non-global numbering version.  The idea being don't port everything over
from the old sysfs - just the core feature set that non-cdev users need.

Cheers,
Kent.

On Wed, Jul 2, 2025 at 12:12 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> >
> > I tend to not interpret it as adding new features. We really just
> > *move* what exists under a slightly different path when you think
> > about it.
> >
> > So what are you suggesting, remove the `edge` attribute and polling
> > features from the new `value` attribute?
> >
>
> Exactly. I'm not suggesting ANY changes to the old sysfs, only your new
> non-global numbering version.  The idea being don't port everything over
> from the old sysfs - just the core feature set that non-cdev users need.
>

I mean, if someone shows up saying they need this or that from the old
sysfs and without they won't switch, we can always add it back I
guess... Much easier than removing something that's carved in stone.

Anything else should go away? `active_low`?

Bart

On Wed, Jul 02, 2025 at 12:28:01PM +0200, Bartosz Golaszewski wrote:
> On Wed, Jul 2, 2025 at 12:12 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > >
> > > I tend to not interpret it as adding new features. We really just
> > > *move* what exists under a slightly different path when you think
> > > about it.
> > >
> > > So what are you suggesting, remove the `edge` attribute and polling
> > > features from the new `value` attribute?
> > >
> >
> > Exactly. I'm not suggesting ANY changes to the old sysfs, only your new
> > non-global numbering version.  The idea being don't port everything over
> > from the old sysfs - just the core feature set that non-cdev users need.
> >
>
> I mean, if someone shows up saying they need this or that from the old
> sysfs and without they won't switch, we can always add it back I
> guess... Much easier than removing something that's carved in stone.
>

Exactly - expect to be supporting whatever goes in now forever.

> Anything else should go away? `active_low`?
>

I don't personally see any value in 'active_low' in the sysfs API if you
drop edges. It is easy enough to flip values as necessary in userspace.
(From time to time I think it should've been dropped from cdev in v2 but, as
above, it is carved in stone now so oh well...)

Cheers,
Kent.

On Wed, 2 Jul 2025 at 13:01, Kent Gibson <warthog618@gmail.com> wrote:
> On Wed, Jul 02, 2025 at 12:28:01PM +0200, Bartosz Golaszewski wrote:
> > On Wed, Jul 2, 2025 at 12:12 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > >
> > > > I tend to not interpret it as adding new features. We really just
> > > > *move* what exists under a slightly different path when you think
> > > > about it.
> > > >
> > > > So what are you suggesting, remove the `edge` attribute and polling
> > > > features from the new `value` attribute?
> > > >
> > >
> > > Exactly. I'm not suggesting ANY changes to the old sysfs, only your new
> > > non-global numbering version.  The idea being don't port everything over
> > > from the old sysfs - just the core feature set that non-cdev users need.
> > >
> >
> > I mean, if someone shows up saying they need this or that from the old
> > sysfs and without they won't switch, we can always add it back I
> > guess... Much easier than removing something that's carved in stone.
>
> Exactly - expect to be supporting whatever goes in now forever.
>
> > Anything else should go away? `active_low`?
> >
>
> I don't personally see any value in 'active_low' in the sysfs API if you
> drop edges. It is easy enough to flip values as necessary in userspace.
> (From time to time I think it should've been dropped from cdev in v2 but, as
> above, it is carved in stone now so oh well...)

IMHO active_low is only really useful if you have some hardware
description that provides it, at which point you may be better off
having a real Linux
driver for the thing connected to the GPIO...

People who mess with GPIO /sysfs better know their hardware,
so they should be aware of the polarity.

From my toolbox:
  - For pcf857x (which is pseudo-bi-directional), I usually just set
    direction to "in" (pulled high) or "out" (driven low),
  - For everything else, switching direction to "out" is ill-defined,
    so I do not write to ".../value", but set direction and value
    together by writing "high" or "low" to ".../direction",
  - For reading, I do use ".../value", of course.

My 0.02€...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

On Wed, Jul 02, 2025 at 07:01:27PM +0800, Kent Gibson wrote:
> On Wed, Jul 02, 2025 at 12:28:01PM +0200, Bartosz Golaszewski wrote:
> > On Wed, Jul 2, 2025 at 12:12 PM Kent Gibson <warthog618@gmail.com> wrote:
> > >
> > > > I tend to not interpret it as adding new features. We really just
> > > > *move* what exists under a slightly different path when you think
> > > > about it.
> > > >
> > > > So what are you suggesting, remove the `edge` attribute and polling
> > > > features from the new `value` attribute?
> > >
> > > Exactly. I'm not suggesting ANY changes to the old sysfs, only your new
> > > non-global numbering version.  The idea being don't port everything over
> > > from the old sysfs - just the core feature set that non-cdev users need.
> >
> > I mean, if someone shows up saying they need this or that from the old
> > sysfs and without they won't switch, we can always add it back I
> > guess... Much easier than removing something that's carved in stone.
> 
> Exactly - expect to be supporting whatever goes in now forever.

+1, this is my biggest worries about the interfaces proposed by this series.

> > Anything else should go away? `active_low`?
> 
> I don't personally see any value in 'active_low' in the sysfs API if you
> drop edges. It is easy enough to flip values as necessary in userspace.
> (From time to time I think it should've been dropped from cdev in v2 but, as
> above, it is carved in stone now so oh well...)

But in cdev case this is different. Active-low state is needed to be
HW independent. For sysfs I agree as it's _already_ HW *dependent*
(due to global number space in use at bare minumum).


-- 
With Best Regards,
Andy Shevchenko

On Wed, Jul 02, 2025 at 02:59:43PM +0300, Andy Shevchenko wrote:
> On Wed, Jul 02, 2025 at 07:01:27PM +0800, Kent Gibson wrote:
> > On Wed, Jul 02, 2025 at 12:28:01PM +0200, Bartosz Golaszewski wrote:
> > > On Wed, Jul 2, 2025 at 12:12 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > > I tend to not interpret it as adding new features. We really just
> > > > > *move* what exists under a slightly different path when you think
> > > > > about it.
> > > > >
> > > > > So what are you suggesting, remove the `edge` attribute and polling
> > > > > features from the new `value` attribute?
> > > >
> > > > Exactly. I'm not suggesting ANY changes to the old sysfs, only your new
> > > > non-global numbering version.  The idea being don't port everything over
> > > > from the old sysfs - just the core feature set that non-cdev users need.
> > >
> > > I mean, if someone shows up saying they need this or that from the old
> > > sysfs and without they won't switch, we can always add it back I
> > > guess... Much easier than removing something that's carved in stone.
> >
> > Exactly - expect to be supporting whatever goes in now forever.
>
> +1, this is my biggest worries about the interfaces proposed by this series.
>
> > > Anything else should go away? `active_low`?
> >
> > I don't personally see any value in 'active_low' in the sysfs API if you
> > drop edges. It is easy enough to flip values as necessary in userspace.
> > (From time to time I think it should've been dropped from cdev in v2 but, as
> > above, it is carved in stone now so oh well...)
>
> But in cdev case this is different. Active-low state is needed to be
> HW independent. For sysfs I agree as it's _already_ HW *dependent*
> (due to global number space in use at bare minumum).
>

As Geert pointed out, userspace needs to set the 'active_low' itself, so
it isn't really providing hardware independence.

The issue isn't whether the logical/physical mapping should exist, but
whether there is any benefit having it baked into the API.
It could've been added in userspace instead and made the uAPI a tad simpler
with one less source of confusion.

Cheers,
Kent.