From: Johan Hovold <johan@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "Markus Pargmann" <mpa@pengutronix.de>,
"Alexandre Courbot" <gnurou@gmail.com>,
"Arun Bharadwaj" <arun@gumstix.com>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Johan Hovold" <johan@kernel.org>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"Sascha Hauer" <kernel@pengutronix.de>
Subject: Re: [PATCH 5/6] gpiolib-sysfs: Show gpio-name in /sys/class/gpio/gpio*/name
Date: Wed, 23 Sep 2015 15:20:51 -0700 [thread overview]
Message-ID: <20150923222051.GD19608@localhost> (raw)
In-Reply-To: <CACRpkdZCSwWUuhbe2+nrEPTRnqNGvJrrbZLDwy8fkHeEJDujUg@mail.gmail.com>
On Mon, Aug 10, 2015 at 11:50:16AM +0200, Linus Walleij wrote:
> On Tue, Aug 4, 2015 at 11:23 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
>
> > This patch adds a sysfs attribute 'name' to gpios that were exported. It
> > exposes the newly added name property of gpio descriptors.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
>
> This needs to also patch Documentation/ABI/testing/sysfs-gpio
> if we should go with it. It says this:
>
> /sys/class/gpio
> /export ... asks the kernel to export a GPIO to userspace
> /unexport ... to return a GPIO to the kernel
> /gpioN ... for each exported GPIO #N
> /value ... always readable, writes fail for input GPIOs
> /direction ... r/w as: in, out (default low); write: high, low
> /edge ... r/w as: none, falling, rising, both
>
> Anyways I don't know if this is right, and that ABI doc is also lying.
>
> Look at this in gpiolib-sysfs.c:
>
> if (chip->names && chip->names[offset])
> ioname = chip->names[offset];
>
> dev = device_create_with_groups(&gpio_class, chip->dev,
> MKDEV(0, 0), data, gpio_groups,
> ioname ? ioname : "gpio%u",
> desc_to_gpio(desc));
>
> I.e. what the ABI doc say about the dirs being named "gpioN" is
> a plain lie, it can have a descriptive name as its directory name
> under /sys/class/gpio/foo-line or so.
>
> Since this already exist and is a flat namespace ... we should
> use that.
>
> However it has the implication like I said before that two names
> cannot be the same. I think Johan's comment that they could
> be non-unique did not take into account the fact that two chips
> could use the same .names array (and that would already fail,
> by the way) so the .names in the struct gpio_chip *MUST* be
> unique as compared to all other names.
It's is unfortunate that we need to potentially maintain this forever,
but it does not mean that we should not allow non-unique names in DT
going forward.
> We *could* deprecate the old line naming mechanism (that create
> dirs named after the pin), and from here on only use gpioN and
> "name" in a separate file like this patch does. However that is
> not really OK either: we want to move away from the GPIO numbers
> and to descriptors and descriptive names, so I currently feel
> we should name the directories after the line instead, and
> require them to be unique.
We have already moved away from the numbers through the introduction of
descriptors. What remains to define is a sane userspace interface. Until
then, let the sysfs-interface be as cumbersome to use as it always has
rather than introducing more (intermediate) ABI to maintain.
Johan
WARNING: multiple messages have this Message-ID (diff)
From: johan@kernel.org (Johan Hovold)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 5/6] gpiolib-sysfs: Show gpio-name in /sys/class/gpio/gpio*/name
Date: Wed, 23 Sep 2015 15:20:51 -0700 [thread overview]
Message-ID: <20150923222051.GD19608@localhost> (raw)
In-Reply-To: <CACRpkdZCSwWUuhbe2+nrEPTRnqNGvJrrbZLDwy8fkHeEJDujUg@mail.gmail.com>
On Mon, Aug 10, 2015 at 11:50:16AM +0200, Linus Walleij wrote:
> On Tue, Aug 4, 2015 at 11:23 AM, Markus Pargmann <mpa@pengutronix.de> wrote:
>
> > This patch adds a sysfs attribute 'name' to gpios that were exported. It
> > exposes the newly added name property of gpio descriptors.
> >
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
>
> This needs to also patch Documentation/ABI/testing/sysfs-gpio
> if we should go with it. It says this:
>
> /sys/class/gpio
> /export ... asks the kernel to export a GPIO to userspace
> /unexport ... to return a GPIO to the kernel
> /gpioN ... for each exported GPIO #N
> /value ... always readable, writes fail for input GPIOs
> /direction ... r/w as: in, out (default low); write: high, low
> /edge ... r/w as: none, falling, rising, both
>
> Anyways I don't know if this is right, and that ABI doc is also lying.
>
> Look at this in gpiolib-sysfs.c:
>
> if (chip->names && chip->names[offset])
> ioname = chip->names[offset];
>
> dev = device_create_with_groups(&gpio_class, chip->dev,
> MKDEV(0, 0), data, gpio_groups,
> ioname ? ioname : "gpio%u",
> desc_to_gpio(desc));
>
> I.e. what the ABI doc say about the dirs being named "gpioN" is
> a plain lie, it can have a descriptive name as its directory name
> under /sys/class/gpio/foo-line or so.
>
> Since this already exist and is a flat namespace ... we should
> use that.
>
> However it has the implication like I said before that two names
> cannot be the same. I think Johan's comment that they could
> be non-unique did not take into account the fact that two chips
> could use the same .names array (and that would already fail,
> by the way) so the .names in the struct gpio_chip *MUST* be
> unique as compared to all other names.
It's is unfortunate that we need to potentially maintain this forever,
but it does not mean that we should not allow non-unique names in DT
going forward.
> We *could* deprecate the old line naming mechanism (that create
> dirs named after the pin), and from here on only use gpioN and
> "name" in a separate file like this patch does. However that is
> not really OK either: we want to move away from the GPIO numbers
> and to descriptors and descriptive names, so I currently feel
> we should name the directories after the line instead, and
> require them to be unique.
We have already moved away from the numbers through the introduction of
descriptors. What remains to define is a sane userspace interface. Until
then, let the sysfs-interface be as cumbersome to use as it always has
rather than introducing more (intermediate) ABI to maintain.
Johan
next prev parent reply other threads:[~2015-09-23 22:19 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-04 9:23 [PATCH 0/6] gpiolib: Add GPIO name support Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-04 9:23 ` [PATCH 1/6] gpiolib-of: Rename gpio_hog functions to be generic Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-10 9:29 ` Linus Walleij
2015-08-10 9:29 ` Linus Walleij
2015-08-13 13:14 ` Markus Pargmann
2015-08-13 13:14 ` Markus Pargmann
2015-08-04 9:23 ` [PATCH 2/6] gpio: Introduce gpio descriptor 'name' Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-10 9:37 ` Linus Walleij
2015-08-10 9:37 ` Linus Walleij
2015-08-13 13:28 ` Markus Pargmann
2015-08-13 13:28 ` Markus Pargmann
2015-08-25 14:25 ` Linus Walleij
2015-08-25 14:25 ` Linus Walleij
2015-08-04 9:23 ` [PATCH 3/6] gpiolib-of: Reuse 'line-name' from DT as gpio descriptor name Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-10 9:38 ` Linus Walleij
2015-08-10 9:38 ` Linus Walleij
2015-08-04 9:23 ` [PATCH 4/6] gpiolib-sysfs: Add gpio name parsing for sysfs export Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-10 9:40 ` Linus Walleij
2015-08-10 9:40 ` Linus Walleij
2015-08-04 9:23 ` [PATCH 5/6] gpiolib-sysfs: Show gpio-name in /sys/class/gpio/gpio*/name Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-10 9:50 ` Linus Walleij
2015-08-10 9:50 ` Linus Walleij
2015-08-13 13:43 ` Markus Pargmann
2015-08-13 13:43 ` Markus Pargmann
2015-09-23 22:20 ` Johan Hovold [this message]
2015-09-23 22:20 ` Johan Hovold
2015-08-04 9:23 ` [PATCH 6/6] gpiolib: Add gpio name information to /sys/kernel/debug/gpio Markus Pargmann
2015-08-04 9:23 ` Markus Pargmann
2015-08-10 10:02 ` Linus Walleij
2015-08-10 10:02 ` Linus Walleij
2015-08-17 7:29 ` Alexandre Courbot
2015-08-17 7:29 ` Alexandre Courbot
2015-08-25 14:34 ` Linus Walleij
2015-08-25 14:34 ` Linus Walleij
2015-08-04 9:25 ` [PATCH 0/6 v2] gpiolib: Add GPIO name support Markus Pargmann
2015-08-04 9:25 ` Markus Pargmann
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=20150923222051.GD19608@localhost \
--to=johan@kernel.org \
--cc=arun@gumstix.com \
--cc=gnurou@gmail.com \
--cc=kernel@pengutronix.de \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=mpa@pengutronix.de \
--cc=u.kleine-koenig@pengutronix.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.