linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bartosz Golaszewski <brgl@bgdev.pl>
To: "Enrico Weigelt, metux IT consult" <lkml@metux.net>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	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 17:26:11 +0100	[thread overview]
Message-ID: <CAMRc=McHdtrvkLvT67ew1+88iPtRpd+MO1+5Tr3YsuLceQor1A@mail.gmail.com> (raw)
In-Reply-To: <0058e57c-5765-3944-3137-10b780985a36@metux.net>

śr., 4 gru 2019 o 13:06 Enrico Weigelt, metux IT consult
<lkml@metux.net> napisał(a):
>
> 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)
>

Nah, you're overreacting, nobody is putting anything on the network.
Most of my work isn't in upstream kernel development - it's actually
launching CE products. And it's been a long time since I've worked
with a client that could be convinced not to use systemd and dbus on
an embedded system. This just makes things so easy for people writing
high-level application code that it's become de-facto standard -
whether we like it or not.

> 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
>

This is just your personal anecdote. I could bring up several examples
of good implementations of event-based, dbus-oriented systems all
right. Bad programmers don't make tools they're using bad.

As for the sysfs vs chardev: this has been discussed a lot. We won't
be adding any new features to the sysfs interface. I want to add this
new feature to allow for creating a central entity controlling GPIO
lines in the user-space. In case of sysfs - IT IS such a central
entity except that it lives in the kernel. It doesn't need to be
notified about changes in line properties.

Bartosz

> > 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

  reply	other threads:[~2019-12-04 16:26 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
2019-12-04 16:26       ` Bartosz Golaszewski [this message]
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='CAMRc=McHdtrvkLvT67ew1+88iPtRpd+MO1+5Tr3YsuLceQor1A@mail.gmail.com' \
    --to=brgl@bgdev.pl \
    --cc=bgolaszewski@baylibre.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkml@metux.net \
    --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).