All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: "Bartosz Golaszewski" <brgl@bgdev.pl>,
	"Ahmad Fatoum" <a.fatoum@pengutronix.de>,
	"Jan Lübbe" <jlu@pengutronix.de>, "Marek Vasut" <marex@denx.de>,
	"Geert Uytterhoeven" <geert+renesas@glider.be>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Bartosz Golaszewski" <bartosz.golaszewski@linaro.org>
Subject: Re: [PATCH v3 00/10] gpio: sysfs: add a per-chip export/unexport attribute pair
Date: Wed, 2 Jul 2025 11:54:39 +0800	[thread overview]
Message-ID: <20250702035439.GA20273@rigel> (raw)
In-Reply-To: <aGPrFnDxG4W7S9Ym@smile.fi.intel.com>

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.



  reply	other threads:[~2025-07-02  3:54 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-30 12:37 [PATCH v3 00/10] gpio: sysfs: add a per-chip export/unexport attribute pair Bartosz Golaszewski
2025-06-30 12:37 ` [PATCH v3 01/10] gpio: sysfs: use gpiod_is_equal() to compare GPIO descriptors Bartosz Golaszewski
2025-07-04  8:05   ` Linus Walleij
2025-06-30 12:37 ` [PATCH v3 02/10] gpio: sysfs: add a parallel class device for each GPIO chip using device IDs Bartosz Golaszewski
2025-06-30 12:37 ` [PATCH v3 03/10] gpio: sysfs: only get the dirent reference for the value attr once Bartosz Golaszewski
2025-07-04  8:05   ` Linus Walleij
2025-06-30 12:37 ` [PATCH v3 04/10] gpio: sysfs: pass gpiod_data directly to internal GPIO sysfs functions Bartosz Golaszewski
2025-06-30 12:37 ` [PATCH v3 05/10] gpio: sysfs: rename the data variable in gpiod_(un)export() Bartosz Golaszewski
2025-06-30 12:37 ` [PATCH v3 06/10] gpio: sysfs: don't use driver data in sysfs callbacks for line attributes Bartosz Golaszewski
2025-06-30 13:27   ` Andy Shevchenko
2025-06-30 13:39     ` Bartosz Golaszewski
2025-06-30 14:22       ` Andy Shevchenko
2025-06-30 12:37 ` [PATCH v3 07/10] gpio: sysfs: don't look up exported lines as class devices Bartosz Golaszewski
2025-06-30 12:37 ` [PATCH v3 08/10] gpio: sysfs: export the GPIO directory locally in the gpiochip<id> directory Bartosz Golaszewski
2025-07-04  8:06   ` Linus Walleij
2025-06-30 12:37 ` [PATCH v3 09/10] gpio: sysfs: allow disabling the legacy parts of the GPIO sysfs interface Bartosz Golaszewski
2025-06-30 12:37 ` [PATCH v3 10/10] gpio: TODO: remove the task for the sysfs rework Bartosz Golaszewski
2025-07-04  8:09   ` Linus Walleij
2025-07-01 14:05 ` [PATCH v3 00/10] gpio: sysfs: add a per-chip export/unexport attribute pair Andy Shevchenko
2025-07-02  3:54   ` Kent Gibson [this message]
2025-07-02  9:45     ` Bartosz Golaszewski
2025-07-02 10:12       ` Kent Gibson
2025-07-02 10:28         ` Bartosz Golaszewski
2025-07-02 11:01           ` Kent Gibson
2025-07-02 11:59             ` Andy Shevchenko
2025-07-02 12:42               ` Kent Gibson
2025-07-02 12:05             ` Geert Uytterhoeven
2025-07-02 11:55       ` Andy Shevchenko
2025-07-02 13:18         ` Bartosz Golaszewski
2025-07-04  8:21           ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250702035439.GA20273@rigel \
    --to=warthog618@gmail.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=andriy.shevchenko@intel.com \
    --cc=bartosz.golaszewski@linaro.org \
    --cc=brgl@bgdev.pl \
    --cc=geert+renesas@glider.be \
    --cc=jlu@pengutronix.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marex@denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.