diff mbox

gpio/sysfs: Add ID parameter for GPIO lines

Message ID 1450370892-2517-1-git-send-email-ricardo.ribalda@gmail.com
State New
Headers show

Commit Message

Ricardo Ribalda Delgado Dec. 17, 2015, 4:48 p.m. UTC
On named GPIOs there is currently no find out their numerical id.

Because of this, a GPIO pin named like:
/sys/class/gpio/PROG_B

cannot be unexported by an application different than the one that
exported it.

This patch adds a new parameter to the GPIO line called id, that
shows the numerical id of a given GPIO. E.g.:

$ cat /sys/class/gpio/PROG_B/id
496
$ echo 496 > unexport

Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
 Documentation/ABI/testing/sysfs-gpio |  1 +
 Documentation/gpio/sysfs.txt         |  3 +++
 drivers/gpio/gpiolib-sysfs.c         | 10 ++++++++++
 3 files changed, 14 insertions(+)

Comments

Ricardo Ribalda Delgado Jan. 26, 2016, 10:25 a.m. UTC | #1
Hello!

Any comment on this?


Thanks!

On Thu, Dec 17, 2015 at 5:48 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> On named GPIOs there is currently no find out their numerical id.
>
> Because of this, a GPIO pin named like:
> /sys/class/gpio/PROG_B
>
> cannot be unexported by an application different than the one that
> exported it.
>
> This patch adds a new parameter to the GPIO line called id, that
> shows the numerical id of a given GPIO. E.g.:
>
> $ cat /sys/class/gpio/PROG_B/id
> 496
> $ echo 496 > unexport
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-gpio |  1 +
>  Documentation/gpio/sysfs.txt         |  3 +++
>  drivers/gpio/gpiolib-sysfs.c         | 10 ++++++++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-gpio b/Documentation/ABI/testing/sysfs-gpio
> index 55ffa2df1c10..53d78abef4f3 100644
> --- a/Documentation/ABI/testing/sysfs-gpio
> +++ b/Documentation/ABI/testing/sysfs-gpio
> @@ -21,6 +21,7 @@ Description:
>             /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
> +           /id ... r only, GPIO number
>         /gpiochipN ... for each gpiochip; #N is its first GPIO
>             /base ... (r/o) same as N
>             /label ... (r/o) descriptive, not necessarily unique
> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
> index aeab01aa4d00..68e1b16e747b 100644
> --- a/Documentation/gpio/sysfs.txt
> +++ b/Documentation/gpio/sysfs.txt
> @@ -96,6 +96,9 @@ and have the following read/write attributes:
>                 for "rising" and "falling" edges will follow this
>                 setting.
>
> +       "id" ... ID used for export/unxexport the GPIO. This value may
> +               be used by named GPIOs, to find out their numerical id.
> +
>  GPIO controllers have paths like /sys/class/gpio/gpiochip42/ (for the
>  controller implementing GPIOs starting at #42) and have the following
>  read-only attributes:
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index b57ed8e55ab5..3fc5c7231aae 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -350,6 +350,15 @@ static ssize_t active_low_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(active_low);
>
> +static ssize_t id_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct gpiod_data *data = dev_get_drvdata(dev);
> +
> +       return  sprintf(buf, "%d\n", desc_to_gpio(data->desc));
> +}
> +static DEVICE_ATTR_RO(id);
> +
>  static umode_t gpio_is_visible(struct kobject *kobj, struct attribute *attr,
>                                int n)
>  {
> @@ -377,6 +386,7 @@ static struct attribute *gpio_attrs[] = {
>         &dev_attr_edge.attr,
>         &dev_attr_value.attr,
>         &dev_attr_active_low.attr,
> +       &dev_attr_id.attr,
>         NULL,
>  };
>
> --
> 2.6.4
>
Alexandre Courbot Feb. 8, 2016, 2:50 a.m. UTC | #2
On Tue, Jan 26, 2016 at 7:25 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:
> Hello!
>
> Any comment on this?

Oops, sorry for the delay...

>
>
> Thanks!
>
> On Thu, Dec 17, 2015 at 5:48 PM, Ricardo Ribalda Delgado
> <ricardo.ribalda@gmail.com> wrote:
>> On named GPIOs there is currently no find out their numerical id.
>>
>> Because of this, a GPIO pin named like:
>> /sys/class/gpio/PROG_B
>>
>> cannot be unexported by an application different than the one that
>> exported it.

On the other hand, do you want other applications to be able to
arbitrarily unexport GPIOs they don't own? Well, it's not like it is
not already possible anyway.

>>
>> This patch adds a new parameter to the GPIO line called id, that
>> shows the numerical id of a given GPIO. E.g.:
>>
>> $ cat /sys/class/gpio/PROG_B/id
>> 496
>> $ echo 496 > unexport

What we *really* ought to do is have GPIOs behave like other resources
and be automatically freed when the application using them exits. This
will be made possible by the chardev interface, but is impossible to
achieve using sysfs by design.

Adding Linus to get his thoughts, but this simple read-only attribute
does make sense to me.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

>>
>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>> ---
>>  Documentation/ABI/testing/sysfs-gpio |  1 +
>>  Documentation/gpio/sysfs.txt         |  3 +++
>>  drivers/gpio/gpiolib-sysfs.c         | 10 ++++++++++
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-gpio b/Documentation/ABI/testing/sysfs-gpio
>> index 55ffa2df1c10..53d78abef4f3 100644
>> --- a/Documentation/ABI/testing/sysfs-gpio
>> +++ b/Documentation/ABI/testing/sysfs-gpio
>> @@ -21,6 +21,7 @@ Description:
>>             /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
>> +           /id ... r only, GPIO number
>>         /gpiochipN ... for each gpiochip; #N is its first GPIO
>>             /base ... (r/o) same as N
>>             /label ... (r/o) descriptive, not necessarily unique
>> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
>> index aeab01aa4d00..68e1b16e747b 100644
>> --- a/Documentation/gpio/sysfs.txt
>> +++ b/Documentation/gpio/sysfs.txt
>> @@ -96,6 +96,9 @@ and have the following read/write attributes:
>>                 for "rising" and "falling" edges will follow this
>>                 setting.
>>
>> +       "id" ... ID used for export/unxexport the GPIO. This value may
>> +               be used by named GPIOs, to find out their numerical id.
>> +
>>  GPIO controllers have paths like /sys/class/gpio/gpiochip42/ (for the
>>  controller implementing GPIOs starting at #42) and have the following
>>  read-only attributes:
>> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
>> index b57ed8e55ab5..3fc5c7231aae 100644
>> --- a/drivers/gpio/gpiolib-sysfs.c
>> +++ b/drivers/gpio/gpiolib-sysfs.c
>> @@ -350,6 +350,15 @@ static ssize_t active_low_store(struct device *dev,
>>  }
>>  static DEVICE_ATTR_RW(active_low);
>>
>> +static ssize_t id_show(struct device *dev,
>> +               struct device_attribute *attr, char *buf)
>> +{
>> +       struct gpiod_data *data = dev_get_drvdata(dev);
>> +
>> +       return  sprintf(buf, "%d\n", desc_to_gpio(data->desc));
>> +}
>> +static DEVICE_ATTR_RO(id);
>> +
>>  static umode_t gpio_is_visible(struct kobject *kobj, struct attribute *attr,
>>                                int n)
>>  {
>> @@ -377,6 +386,7 @@ static struct attribute *gpio_attrs[] = {
>>         &dev_attr_edge.attr,
>>         &dev_attr_value.attr,
>>         &dev_attr_active_low.attr,
>> +       &dev_attr_id.attr,
>>         NULL,
>>  };
>>
>> --
>> 2.6.4
>>
>
>
>
> --
> Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij Feb. 15, 2016, 7:11 p.m. UTC | #3
On Thu, Dec 17, 2015 at 5:48 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@gmail.com> wrote:

> On named GPIOs there is currently no find out their numerical id.
>
> Because of this, a GPIO pin named like:
> /sys/class/gpio/PROG_B
>
> cannot be unexported by an application different than the one that
> exported it.
>
> This patch adds a new parameter to the GPIO line called id, that
> shows the numerical id of a given GPIO. E.g.:
>
> $ cat /sys/class/gpio/PROG_B/id
> 496
> $ echo 496 > unexport
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>

NACK. I have just merged commits marking the entire sysfs
ABI deprecated. It will *NOT* be developed. Please help out in
evolving the character device instead.

c.f.
http://marc.info/?l=linux-gpio&m=145494198509404&w=2
http://marc.info/?l=linux-gpio&m=145554244502862&w=2

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/ABI/testing/sysfs-gpio b/Documentation/ABI/testing/sysfs-gpio
index 55ffa2df1c10..53d78abef4f3 100644
--- a/Documentation/ABI/testing/sysfs-gpio
+++ b/Documentation/ABI/testing/sysfs-gpio
@@ -21,6 +21,7 @@  Description:
 	    /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
+	    /id ... r only, GPIO number
 	/gpiochipN ... for each gpiochip; #N is its first GPIO
 	    /base ... (r/o) same as N
 	    /label ... (r/o) descriptive, not necessarily unique
diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
index aeab01aa4d00..68e1b16e747b 100644
--- a/Documentation/gpio/sysfs.txt
+++ b/Documentation/gpio/sysfs.txt
@@ -96,6 +96,9 @@  and have the following read/write attributes:
 		for "rising" and "falling" edges will follow this
 		setting.
 
+	"id" ... ID used for export/unxexport the GPIO. This value may
+		be used by named GPIOs, to find out their numerical id.
+
 GPIO controllers have paths like /sys/class/gpio/gpiochip42/ (for the
 controller implementing GPIOs starting at #42) and have the following
 read-only attributes:
diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index b57ed8e55ab5..3fc5c7231aae 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -350,6 +350,15 @@  static ssize_t active_low_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(active_low);
 
+static ssize_t id_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct gpiod_data *data = dev_get_drvdata(dev);
+
+	return  sprintf(buf, "%d\n", desc_to_gpio(data->desc));
+}
+static DEVICE_ATTR_RO(id);
+
 static umode_t gpio_is_visible(struct kobject *kobj, struct attribute *attr,
 			       int n)
 {
@@ -377,6 +386,7 @@  static struct attribute *gpio_attrs[] = {
 	&dev_attr_edge.attr,
 	&dev_attr_value.attr,
 	&dev_attr_active_low.attr,
+	&dev_attr_id.attr,
 	NULL,
 };