diff mbox series

[RFC/RFT,06/15] gpio: sysfs: remove the mockdev pointer from struct gpio_device

Message ID 20250610-gpio-sysfs-chip-export-v1-6-a8c7aa4478b1@linaro.org
State New
Headers show
Series gpio: sysfs: add a per-chip export/unexport attribute pair | expand

Commit Message

Bartosz Golaszewski June 10, 2025, 2:38 p.m. UTC 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

The usage of the mockdev pointer in struct gpio_device is limited to the
GPIO sysfs code. There's no reason to keep it in this top-level
structure. Create a separate structure containing the reference to the
GPIO device and the dummy class device that will be passed to
device_create_with_groups(). The !gdev->mockdev checks can be removed as
long as we make sure that all operations on the GPIO class are protected
with the sysfs lock.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-sysfs.c | 81 +++++++++++++++++++++++++++++---------------
 drivers/gpio/gpiolib.h       |  3 --
 2 files changed, 53 insertions(+), 31 deletions(-)

Comments

Linus Walleij June 11, 2025, 8:19 a.m. UTC | #1 
On Tue, Jun 10, 2025 at 4:38 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> The usage of the mockdev pointer in struct gpio_device is limited to the
> GPIO sysfs code. There's no reason to keep it in this top-level
> structure. Create a separate structure containing the reference to the
> GPIO device and the dummy class device that will be passed to
> device_create_with_groups(). The !gdev->mockdev checks can be removed as
> long as we make sure that all operations on the GPIO class are protected
> with the sysfs lock.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Much better like this!

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index f23b4efea5905a9eab51ed9e50b5159135a8e26c..956411fc467a26a9827c616d8dc067c70f9244bf 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -43,6 +43,11 @@  struct gpiod_data {
 	bool direction_can_change;
 };
 
+struct gpiodev_data {
+	struct gpio_device *gdev;
+	struct device *cdev_base; /* Class device by GPIO base */
+};
+
 /*
  * Lock to serialise gpiod export and unexport, and prevent re-export of
  * gpiod whose chip is being unregistered.
@@ -399,27 +404,27 @@  static const struct attribute_group *gpio_groups[] = {
 static ssize_t base_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
-	const struct gpio_device *gdev = dev_get_drvdata(dev);
+	const struct gpiodev_data *data = dev_get_drvdata(dev);
 
-	return sysfs_emit(buf, "%u\n", gdev->base);
+	return sysfs_emit(buf, "%u\n", data->gdev->base);
 }
 static DEVICE_ATTR_RO(base);
 
 static ssize_t label_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
-	const struct gpio_device *gdev = dev_get_drvdata(dev);
+	const struct gpiodev_data *data = dev_get_drvdata(dev);
 
-	return sysfs_emit(buf, "%s\n", gdev->label);
+	return sysfs_emit(buf, "%s\n", data->gdev->label);
 }
 static DEVICE_ATTR_RO(label);
 
 static ssize_t ngpio_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
-	const struct gpio_device *gdev = dev_get_drvdata(dev);
+	const struct gpiodev_data *data = dev_get_drvdata(dev);
 
-	return sysfs_emit(buf, "%u\n", gdev->ngpio);
+	return sysfs_emit(buf, "%u\n", data->gdev->ngpio);
 }
 static DEVICE_ATTR_RO(ngpio);
 
@@ -545,6 +550,26 @@  static const struct class gpio_class = {
 	.class_groups =	gpio_class_groups,
 };
 
+static int match_gdev(struct device *dev, const void *desc)
+{
+	struct gpiodev_data *data = dev_get_drvdata(dev);
+	const struct gpio_device *gdev = desc;
+
+	return data && data->gdev == gdev;
+}
+
+static struct gpiodev_data *
+gdev_get_data(struct gpio_device *gdev) __must_hold(&sysfs_lock)
+{
+	struct device *cdev __free(put_device) = class_find_device(&gpio_class,
+								   NULL, gdev,
+								   match_gdev);
+	if (!cdev)
+		return NULL;
+
+	return dev_get_drvdata(cdev);
+};
+
 /**
  * gpiod_export - export a GPIO through sysfs
  * @desc: GPIO to make available, already requested
@@ -590,12 +615,6 @@  int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
 
 	guard(mutex)(&sysfs_lock);
 
-	/* check if chip is being removed */
-	if (!gdev->mockdev) {
-		status = -ENODEV;
-		goto err_clear_bit;
-	}
-
 	if (!test_bit(FLAG_REQUESTED, &desc->flags)) {
 		gpiod_dbg(desc, "%s: unavailable (not requested)\n", __func__);
 		status = -EPERM;
@@ -719,9 +738,9 @@  EXPORT_SYMBOL_GPL(gpiod_unexport);
 
 int gpiochip_sysfs_register(struct gpio_device *gdev)
 {
+	struct gpiodev_data *data;
 	struct gpio_chip *chip;
 	struct device *parent;
-	struct device *dev;
 
 	/*
 	 * Many systems add gpio chips for SOC support very early,
@@ -747,32 +766,41 @@  int gpiochip_sysfs_register(struct gpio_device *gdev)
 	else
 		parent = &gdev->dev;
 
-	/* use chip->base for the ID; it's already known to be unique */
-	dev = device_create_with_groups(&gpio_class, parent, MKDEV(0, 0), gdev,
-					gpiochip_groups, GPIOCHIP_NAME "%d",
-					chip->base);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
+	data = kmalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->gdev = gdev;
 
 	guard(mutex)(&sysfs_lock);
-	gdev->mockdev = dev;
+
+	/* use chip->base for the ID; it's already known to be unique */
+	data->cdev_base = device_create_with_groups(&gpio_class, parent,
+						    MKDEV(0, 0), data,
+						    gpiochip_groups,
+						    GPIOCHIP_NAME "%d",
+						    chip->base);
+	if (IS_ERR(data->cdev_base)) {
+		kfree(data);
+		return PTR_ERR(data->cdev_base);
+	}
 
 	return 0;
 }
 
 void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 {
+	struct gpiodev_data *data;
 	struct gpio_desc *desc;
 	struct gpio_chip *chip;
 
 	scoped_guard(mutex, &sysfs_lock) {
-		if (!gdev->mockdev)
+		data = gdev_get_data(gdev);
+		if (!data)
 			return;
 
-		device_unregister(gdev->mockdev);
-
-		/* prevent further gpiod exports */
-		gdev->mockdev = NULL;
+		device_unregister(data->cdev_base);
+		kfree(data);
 	}
 
 	guard(srcu)(&gdev->srcu);
@@ -798,9 +826,6 @@  static int gpiofind_sysfs_register(struct gpio_chip *gc, const void *data)
 	struct gpio_device *gdev = gc->gpiodev;
 	int ret;
 
-	if (gdev->mockdev)
-		return 0;
-
 	ret = gpiochip_sysfs_register(gdev);
 	if (ret)
 		chip_err(gc, "failed to register the sysfs entry: %d\n", ret);
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 58f64056de77b05e8cbcb2395a55da793b1a52fa..9b74738a9ca5b1a4826c8d56d871f8a7cf6ea1e7 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -27,8 +27,6 @@ 
  * @dev: the GPIO device struct
  * @chrdev: character device for the GPIO device
  * @id: numerical ID number for the GPIO chip
- * @mockdev: class device used by the deprecated sysfs interface (may be
- * NULL)
  * @owner: helps prevent removal of modules exporting active GPIOs
  * @chip: pointer to the corresponding gpiochip, holding static
  * data for this device
@@ -65,7 +63,6 @@  struct gpio_device {
 	struct device		dev;
 	struct cdev		chrdev;
 	int			id;
-	struct device		*mockdev;
 	struct module		*owner;
 	struct gpio_chip __rcu	*chip;
 	struct gpio_desc	*descs;