linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Enrico Weigelt, metux IT consult" <lkml@metux.net>
To: Bartosz Golaszewski <brgl@bgdev.pl>,
	Linus Walleij <linus.walleij@linaro.org>
Cc: Kent Gibson <warthog618@gmail.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH 0/8] gpiolib: add an ioctl() for monitoring line status changes
Date: Wed, 4 Dec 2019 13:06:14 +0100	[thread overview]
Message-ID: <0058e57c-5765-3944-3137-10b780985a36@metux.net> (raw)
In-Reply-To: <CAMRc=McH6m3Lsvz8g1JSD_c-QNdb-Kh0+8BH5EKcEW2vM2VYJA@mail.gmail.com>

On 29.11.19 11:58, Bartosz Golaszewski wrote:

Hi folks,


missed much of this thread, so please excuse me if I'm going to say
something stupid ;-)

> No, it really is just about keeping the line information in user-space
> synchronized with the one in the kernel without re-reading the line
> info periodically. Whether it's the kernel that requests the line or
> other user-space process doesn't matter. We just want to stay
> up-to-date with the information we already do have access to.

In that case, why not just using inotify+friends or some blocking read
for that ?

Personally, I'm a really big friend of synthentic filesystems instead of
ioctl()s, because they're so simple to use (don't wanna repeat the whole
Plan9 literature here ;-)), so I'd do it in this way:

* have a file in /sys/class/gpio/... holding the current gpio
  information, in some easily parsable format. (either one file
  globally or one per chip)
* a) this file can be monitored via inotify (just one event when
     something had changed, or maybe have mtime reflect the time of
     last change)
* b) allow read after EOF, which is blocking, until something changes

That way, userland implementation would be trivial enough to do it
in a simple shell script.

> It may seem like a feature-creep because this is the third new feature
> being added to the character device in a short span of time. 

Personally, I'm not a fan of such chardevs at all, but this raises
another interesting pretty general topic: sidechannel/out-of-band data
of chardevs.

Originally, Unix chardevs have been designed as something operates on
streams of bytes, maybe with some extra operations (eg. setting baud
rate, etc). ioctl()s have been implemented as some escape route for such
extra functions. Over the many years, there's been a massive ioctl()
inflation, even hardware specific ones. IMHO, this is a huge mess, which
requires to lots of extra work for userland applications, which should
instead get generic interfaces, even leading to ridiculous things like
HAL, etc.

Seriously, we should lean back and think of better ways. One idea would
be making devices more directly-like, where things like oob-streams
or config attributes can be enumerated and addressed by names.
(actually, if I could change history, I'd make them actual directories)

> But at
> the same time: user-space wants to use GPIOs and they're mostly doing
> it over sysfs. If you want people to switch over to the character
> device - we must make it at least as useful as sysfs.

Personally, I don't see any practical reason, why I should use the
chardev at all - sysfs is much simpler. The only reason I see is when
needing to set several lines at once - but I haven't seen practical a
usecase where there isn't some specific device behind them, that
deservers it's own driver, attaching to some suitable subsystem.
(actually, I rarely talk to gpios directly from userland - except for
some lowlevel debugging purposes).

> These new features are not unjustified: I receive a significant amount
> of mail with feature-requests for libgpiod (also from people who are
> not well aware that I can only support features exposed by mainline
> kernel).

Do you have more detailed information on what these folks are really
up to, what their actual usecases are ?

> Last thing that users often complain about is the fact that with the
> character device, the state of GPIO lines used by user-space needs to
> be kept by the process using them. This unfortunately often comes from
> the lack of understanding of how a character device functions, but it
> really seems users want to have a centralized agent that takes control
> of the lines, provides standardized interface to interact with them
> and exports their metadata.

Yeah, I also sometimes have those kinds of clients. So far, in all cases
turned out that they actually needed a more high level device driver
(or extend an existing one) instead of touching gpios directly from
within the application. (usually things like relais or extra uart
signalling lines, power control, etc, via gpio, etc).

> Recognizing this fact, I implemented a proof-of-concept dbus daemon, 
> but one thing that it could really
> benefit from is dynamic, event-based synchronization and this series
> tries to add just that (BTW please take a look at the discussion under
> patch 7/8 - the code in this series will probably change a lot).

Oh, no, I've been afraid that this would be coming ... desktop bus in
embedded system is a real nightmare. (yes, some people are even insane
enough to put that thing on an public IP interface and so make critical
machinery like cards easily remote controllable by average school kids)

Seriously, I've seen a lot of similar constructs in the field, all of
them have quickly turned out as complete crap - usually coming from
folks who need >50kLoc ontop of trivial cansocket with silly excuses
like formal certification :o). Please, don't give these kids a machine
gun, they will just hurt innocent people with it :P

> We should probably start a new thread on this. I'll play the
> user-space's advocate and say: has anyone ever needed this? Do other
> kernel sub-systems do this?

Hotplug could lead to such situations.

Playing the kernel advocate here: better don't even consider opening
this apocalypse box and focus on clean kernel drivers instead :p


--mtx

---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

  parent reply	other threads:[~2019-12-04 12:06 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-27 13:35 [PATCH 0/8] gpiolib: add an ioctl() for monitoring line status changes Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 1/8] gpiolib: use 'unsigned int' instead of 'unsigned' in gpio_set_config() Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 2/8] gpiolib: have a single place of calling set_config() Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 3/8] gpiolib: convert the type of hwnum to unsigned int in gpiochip_get_desc() Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 4/8] gpiolib: use gpiochip_get_desc() in linehandle_create() Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 5/8] gpiolib: use gpiochip_get_desc() in lineevent_create() Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 6/8] gpiolib: actually protect the line event kfifo with mutex Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 7/8] gpiolib: add new ioctl() for monitoring changes in line info Bartosz Golaszewski
2019-11-27 15:24   ` Kent Gibson
2019-11-27 15:50     ` Bartosz Golaszewski
2019-11-27 23:23       ` Kent Gibson
2019-11-28  9:45         ` Bartosz Golaszewski
2019-11-28 14:10           ` Kent Gibson
2019-11-28 14:36             ` Bartosz Golaszewski
2019-11-28 15:02               ` Kent Gibson
2019-11-29  9:43                 ` Bartosz Golaszewski
2019-11-29 13:49                   ` Kent Gibson
2019-12-01 15:25                     ` Bartosz Golaszewski
2019-12-01 23:43                       ` Kent Gibson
2019-12-02 17:11                         ` Bartosz Golaszewski
2019-12-03  2:09                           ` Kent Gibson
2019-12-03  8:58                             ` Bartosz Golaszewski
2019-11-27 13:35 ` [PATCH 8/8] tools: gpio: implement gpio-watch Bartosz Golaszewski
2019-11-29 10:04 ` [PATCH 0/8] gpiolib: add an ioctl() for monitoring line status changes Linus Walleij
2019-11-29 10:58   ` Bartosz Golaszewski
2019-11-29 12:57     ` Linus Walleij
2019-12-04 12:32       ` Enrico Weigelt, metux IT consult
2019-12-04 12:06     ` Enrico Weigelt, metux IT consult [this message]
2019-12-04 16:26       ` Bartosz Golaszewski
2019-12-06 12:52         ` Enrico Weigelt, metux IT consult
2019-12-06 15:57           ` Bartosz Golaszewski
2019-12-06 15:44       ` Linus Walleij
2019-12-06 17:50         ` Enrico Weigelt, metux IT consult

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=0058e57c-5765-3944-3137-10b780985a36@metux.net \
    --to=lkml@metux.net \
    --cc=bgolaszewski@baylibre.com \
    --cc=brgl@bgdev.pl \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=warthog618@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).