gpio: sysfs: add a parallel class device for each GPIO chip using device IDs

In order to enable moving away from the global GPIO numberspace-based
exporting of lines over sysfs: add a parallel, per-chip entry under
/sys/class/gpio/ for every registered GPIO chip, denoted by device ID
in the file name and not its base GPIO number.

Compared to the existing chip group: it does not contain the "base"
attribute as the goal of this change is to not refer to GPIOs by their
global number from user-space anymore. It also contains its own,
per-chip export/unexport attribute pair which allow to export lines by
their hardware offset within the chip.

Caveat #1: the new device cannot be a link to (or be linked to by) the
existing "gpiochip<BASE>" entry as we cannot create links in
/sys/class/xyz/.

Caveat #2: the new entry cannot be named "gpiochipX" as it could
conflict with devices whose base is statically defined to a low number.
Let's go with "chipX" instead.

While at it: the chip label is unique so update the untrue statement
when extending the docs.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20250704-gpio-sysfs-chip-export-v4-2-9289d8758243@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

diff --git a/Documentation/ABI/obsolete/sysfs-gpio b/Documentation/ABI/obsolete/sysfs-gpio
index 480316fee6d80fb7a0ed61706559838591ec0932..ff694708a3bef787afa42dedf94faf209c44dbf0 100644 (file)
--- a/Documentation/ABI/obsolete/sysfs-gpio
+++ b/Documentation/ABI/obsolete/sysfs-gpio
@@ -25,8 +25,13 @@ Description:
            /active_low ... r/w as: 0, 1
        /gpiochipN ... for each gpiochip; #N is its first GPIO
            /base ... (r/o) same as N
-           /label ... (r/o) descriptive, not necessarily unique
+           /label ... (r/o) descriptive chip name
            /ngpio ... (r/o) number of GPIOs; numbered N to N + (ngpio - 1)
+       /chipX ... for each gpiochip; #X is the gpio device ID
+           /export ... asks the kernel to export a GPIO at HW offset X to userspace
+           /unexport ... to return a GPIO at HW offset X to the kernel
+           /label ... (r/o) descriptive chip name
+           /ngpio ... (r/o) number of GPIOs exposed by the chip
 
   This ABI is obsoleted by Documentation/ABI/testing/gpio-cdev and will be
   removed after 2020.

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index c4e85f2827697d0239ff6296caf49d243cf10fe8..990db0405cc86c42bad61295dc823f970199534e 100644 (file)
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -45,6 +45,7 @@ struct gpiod_data {
 
 struct gpiodev_data {
        struct gpio_device *gdev;
+       struct device *cdev_id; /* Class device by GPIO device ID */
        struct device *cdev_base; /* Class device by GPIO base */
 };
 
@@ -399,6 +400,14 @@ static const struct attribute_group *gpio_groups[] = {
  *   /base ... matching gpio_chip.base (N)
  *   /label ... matching gpio_chip.label
  *   /ngpio ... matching gpio_chip.ngpio
+ *
+ * AND
+ *
+ * /sys/class/gpio/chipX/
+ *   /export ... export GPIO at given offset
+ *   /unexport ... unexport GPIO at given offset
+ *   /label ... matching gpio_chip.label
+ *   /ngpio ... matching gpio_chip.ngpio
  */
 
 static ssize_t base_show(struct device *dev, struct device_attribute *attr,
@@ -428,6 +437,111 @@ static ssize_t ngpio_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(ngpio);
 
+static int export_gpio_desc(struct gpio_desc *desc)
+{
+       int offset, ret;
+
+       CLASS(gpio_chip_guard, guard)(desc);
+       if (!guard.gc)
+               return -ENODEV;
+
+       offset = gpio_chip_hwgpio(desc);
+       if (!gpiochip_line_is_valid(guard.gc, offset)) {
+               pr_debug_ratelimited("%s: GPIO %d masked\n", __func__,
+                                    gpio_chip_hwgpio(desc));
+               return -EINVAL;
+       }
+
+       /*
+        * No extra locking here; FLAG_SYSFS just signifies that the
+        * request and export were done by on behalf of userspace, so
+        * they may be undone on its behalf too.
+        */
+
+       ret = gpiod_request_user(desc, "sysfs");
+       if (ret)
+               return ret;
+
+       ret = gpiod_set_transitory(desc, false);
+       if (ret) {
+               gpiod_free(desc);
+               return ret;
+       }
+
+       ret = gpiod_export(desc, true);
+       if (ret < 0) {
+               gpiod_free(desc);
+       } else {
+               set_bit(FLAG_SYSFS, &desc->flags);
+               gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_REQUESTED);
+       }
+
+       return ret;
+}
+
+static int unexport_gpio_desc(struct gpio_desc *desc)
+{
+       /*
+        * No extra locking here; FLAG_SYSFS just signifies that the
+        * request and export were done by on behalf of userspace, so
+        * they may be undone on its behalf too.
+        */
+       if (!test_and_clear_bit(FLAG_SYSFS, &desc->flags))
+               return -EINVAL;
+
+       gpiod_unexport(desc);
+       gpiod_free(desc);
+
+       return 0;
+}
+
+static ssize_t do_chip_export_store(struct device *dev,
+                                   struct device_attribute *attr,
+                                   const char *buf, ssize_t size,
+                                   int (*handler)(struct gpio_desc *desc))
+{
+       struct gpiodev_data *data = dev_get_drvdata(dev);
+       struct gpio_device *gdev = data->gdev;
+       struct gpio_desc *desc;
+       unsigned int gpio;
+       int ret;
+
+       ret = kstrtouint(buf, 0, &gpio);
+       if (ret)
+               return ret;
+
+       desc = gpio_device_get_desc(gdev, gpio);
+       if (IS_ERR(desc))
+               return PTR_ERR(desc);
+
+       ret = handler(desc);
+       if (ret)
+               return ret;
+
+       return size;
+}
+
+static ssize_t chip_export_store(struct device *dev,
+                                struct device_attribute *attr,
+                                const char *buf, size_t size)
+{
+       return do_chip_export_store(dev, attr, buf, size, export_gpio_desc);
+}
+
+static struct device_attribute dev_attr_export = __ATTR(export, 0200, NULL,
+                                                       chip_export_store);
+
+static ssize_t chip_unexport_store(struct device *dev,
+                                  struct device_attribute *attr,
+                                  const char *buf, size_t size)
+{
+       return do_chip_export_store(dev, attr, buf, size, unexport_gpio_desc);
+}
+
+static struct device_attribute dev_attr_unexport = __ATTR(unexport, 0200,
+                                                         NULL,
+                                                         chip_unexport_store);
+
 static struct attribute *gpiochip_attrs[] = {
        &dev_attr_base.attr,
        &dev_attr_label.attr,
@@ -436,6 +550,15 @@ static struct attribute *gpiochip_attrs[] = {
 };
 ATTRIBUTE_GROUPS(gpiochip);
 
+static struct attribute *gpiochip_ext_attrs[] = {
+       &dev_attr_label.attr,
+       &dev_attr_ngpio.attr,
+       &dev_attr_export.attr,
+       &dev_attr_unexport.attr,
+       NULL
+};
+ATTRIBUTE_GROUPS(gpiochip_ext);
+
 /*
  * /sys/class/gpio/export ... write-only
  *     integer N ... number of GPIO to export (full access)
@@ -447,7 +570,7 @@ static ssize_t export_store(const struct class *class,
                            const char *buf, size_t len)
 {
        struct gpio_desc *desc;
-       int status, offset;
+       int status;
        long gpio;
 
        status = kstrtol(buf, 0, &gpio);
@@ -461,40 +584,7 @@ static ssize_t export_store(const struct class *class,
                return -EINVAL;
        }
 
-       CLASS(gpio_chip_guard, guard)(desc);
-       if (!guard.gc)
-               return -ENODEV;
-
-       offset = gpio_chip_hwgpio(desc);
-       if (!gpiochip_line_is_valid(guard.gc, offset)) {
-               pr_debug_ratelimited("%s: GPIO %ld masked\n", __func__, gpio);
-               return -EINVAL;
-       }
-
-       /* No extra locking here; FLAG_SYSFS just signifies that the
-        * request and export were done by on behalf of userspace, so
-        * they may be undone on its behalf too.
-        */
-
-       status = gpiod_request_user(desc, "sysfs");
-       if (status)
-               goto done;
-
-       status = gpiod_set_transitory(desc, false);
-       if (status) {
-               gpiod_free(desc);
-               goto done;
-       }
-
-       status = gpiod_export(desc, true);
-       if (status < 0) {
-               gpiod_free(desc);
-       } else {
-               set_bit(FLAG_SYSFS, &desc->flags);
-               gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_REQUESTED);
-       }
-
-done:
+       status = export_gpio_desc(desc);
        if (status)
                pr_debug("%s: status %d\n", __func__, status);
        return status ? : len;
@@ -511,7 +601,7 @@ static ssize_t unexport_store(const struct class *class,
 
        status = kstrtol(buf, 0, &gpio);
        if (status < 0)
-               goto done;
+               return status;
 
        desc = gpio_to_desc(gpio);
        /* reject bogus commands (gpiod_unexport() ignores them) */
@@ -520,18 +610,7 @@ static ssize_t unexport_store(const struct class *class,
                return -EINVAL;
        }
 
-       status = -EINVAL;
-
-       /* No extra locking here; FLAG_SYSFS just signifies that the
-        * request and export were done by on behalf of userspace, so
-        * they may be undone on its behalf too.
-        */
-       if (test_and_clear_bit(FLAG_SYSFS, &desc->flags)) {
-               gpiod_unexport(desc);
-               gpiod_free(desc);
-               status = 0;
-       }
-done:
+       status = unexport_gpio_desc(desc);
        if (status)
                pr_debug("%s: status %d\n", __func__, status);
        return status ? : len;
@@ -561,6 +640,11 @@ static int match_gdev(struct device *dev, const void *desc)
 static struct gpiodev_data *
 gdev_get_data(struct gpio_device *gdev) __must_hold(&sysfs_lock)
 {
+       /*
+        * Find the first device in GPIO class that matches. Whether that's
+        * the one indexed by GPIO base or device ID doesn't matter, it has
+        * the same address set as driver data.
+        */
        struct device *cdev __free(put_device) = class_find_device(&gpio_class,
                                                                   NULL, gdev,
                                                                   match_gdev);
@@ -787,6 +871,17 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
                return err;
        }
 
+       data->cdev_id = device_create_with_groups(&gpio_class, parent,
+                                                 MKDEV(0, 0), data,
+                                                 gpiochip_ext_groups,
+                                                 "chip%d", gdev->id);
+       if (IS_ERR(data->cdev_id)) {
+               device_unregister(data->cdev_base);
+               err = PTR_ERR(data->cdev_id);
+               kfree(data);
+               return err;
+       }
+
        return 0;
 }
 
@@ -802,6 +897,7 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
                        return;
 
                device_unregister(data->cdev_base);
+               device_unregister(data->cdev_id);
                kfree(data);
        }