gpio: sim: don't fiddle with GPIOLIB private members
authorBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Mon, 11 Sep 2023 11:07:40 +0000 (13:07 +0200)
committerBartosz Golaszewski <bartosz.golaszewski@linaro.org>
Tue, 12 Sep 2023 07:29:24 +0000 (09:29 +0200)
We access internals of struct gpio_device and struct gpio_desc because
it's easier but it can actually be avoided and we're working towards a
better encapsulation of GPIO data structures across the kernel so let's
start at home.

Instead of checking gpio_desc flags, let's just track the requests of
GPIOs in the driver. We also already store the information about
direction of simulated lines.

For kobjects needed by sysfs callbacks: we can iterate over the children
devices of the top-level platform device and compare their fwnodes
against the one passed to the init function from probe.

While at it: fix one line break and remove the untrue part about
configfs callbacks using dev_get_drvdata() from a comment.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
drivers/gpio/gpio-sim.c

index 271db3639a78209e00d45d558d6354bb0161e27f..2b9d9e172d5d60fe36e203d37d42fe3736e97b09 100644 (file)
@@ -12,6 +12,7 @@
 #include <linux/completion.h>
 #include <linux/configfs.h>
 #include <linux/device.h>
+#include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
 #include <linux/gpio/machine.h>
 #include <linux/idr.h>
@@ -30,8 +31,6 @@
 #include <linux/string_helpers.h>
 #include <linux/sysfs.h>
 
-#include "gpiolib.h"
-
 #define GPIO_SIM_NGPIO_MAX     1024
 #define GPIO_SIM_PROP_MAX      4 /* Max 3 properties + sentinel. */
 #define GPIO_SIM_NUM_ATTRS     3 /* value, pull and sentinel */
@@ -40,6 +39,8 @@ static DEFINE_IDA(gpio_sim_ida);
 
 struct gpio_sim_chip {
        struct gpio_chip gc;
+       struct device *dev;
+       unsigned long *request_map;
        unsigned long *direction_map;
        unsigned long *value_map;
        unsigned long *pull_map;
@@ -63,16 +64,11 @@ static int gpio_sim_apply_pull(struct gpio_sim_chip *chip,
                               unsigned int offset, int value)
 {
        int irq, irq_type, ret;
-       struct gpio_desc *desc;
-       struct gpio_chip *gc;
-
-       gc = &chip->gc;
-       desc = &gc->gpiodev->descs[offset];
 
        guard(mutex)(&chip->lock);
 
-       if (test_bit(FLAG_REQUESTED, &desc->flags) &&
-           !test_bit(FLAG_IS_OUT, &desc->flags)) {
+       if (test_bit(offset, chip->request_map) &&
+           test_bit(offset, chip->direction_map)) {
                if (value == !!test_bit(offset, chip->value_map))
                        goto set_pull;
 
@@ -99,8 +95,8 @@ static int gpio_sim_apply_pull(struct gpio_sim_chip *chip,
 
 set_value:
        /* Change the value unless we're actively driving the line. */
-       if (!test_bit(FLAG_REQUESTED, &desc->flags) ||
-           !test_bit(FLAG_IS_OUT, &desc->flags))
+       if (!test_bit(offset, chip->request_map) ||
+           test_bit(offset, chip->direction_map))
                __assign_bit(offset, chip->value_map, value);
 
 set_pull:
@@ -180,8 +176,8 @@ static int gpio_sim_get_direction(struct gpio_chip *gc, unsigned int offset)
        return direction ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;
 }
 
-static int gpio_sim_set_config(struct gpio_chip *gc,
-                                 unsigned int offset, unsigned long config)
+static int gpio_sim_set_config(struct gpio_chip *gc, unsigned int offset,
+                              unsigned long config)
 {
        struct gpio_sim_chip *chip = gpiochip_get_data(gc);
 
@@ -204,13 +200,25 @@ static int gpio_sim_to_irq(struct gpio_chip *gc, unsigned int offset)
        return irq_create_mapping(chip->irq_sim, offset);
 }
 
-static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset)
+static int gpio_sim_request(struct gpio_chip *gc, unsigned int offset)
 {
        struct gpio_sim_chip *chip = gpiochip_get_data(gc);
 
        scoped_guard(mutex, &chip->lock)
+               __set_bit(offset, chip->request_map);
+
+       return 0;
+}
+
+static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset)
+{
+       struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+       scoped_guard(mutex, &chip->lock) {
                __assign_bit(offset, chip->value_map,
                             !!test_bit(offset, chip->pull_map));
+               __clear_bit(offset, chip->request_map);
+       }
 }
 
 static ssize_t gpio_sim_sysfs_val_show(struct device *dev,
@@ -282,6 +290,13 @@ static void gpio_sim_mutex_destroy(void *data)
        mutex_destroy(lock);
 }
 
+static void gpio_sim_put_device(void *data)
+{
+       struct device *dev = data;
+
+       put_device(dev);
+}
+
 static void gpio_sim_dispose_mappings(void *data)
 {
        struct gpio_sim_chip *chip = data;
@@ -295,7 +310,7 @@ static void gpio_sim_sysfs_remove(void *data)
 {
        struct gpio_sim_chip *chip = data;
 
-       sysfs_remove_groups(&chip->gc.gpiodev->dev.kobj, chip->attr_groups);
+       sysfs_remove_groups(&chip->dev->kobj, chip->attr_groups);
 }
 
 static int gpio_sim_setup_sysfs(struct gpio_sim_chip *chip)
@@ -352,14 +367,18 @@ static int gpio_sim_setup_sysfs(struct gpio_sim_chip *chip)
                chip->attr_groups[i] = attr_group;
        }
 
-       ret = sysfs_create_groups(&chip->gc.gpiodev->dev.kobj,
-                                 chip->attr_groups);
+       ret = sysfs_create_groups(&chip->dev->kobj, chip->attr_groups);
        if (ret)
                return ret;
 
        return devm_add_action_or_reset(dev, gpio_sim_sysfs_remove, chip);
 }
 
+static int gpio_sim_dev_match_fwnode(struct device *dev, void *data)
+{
+       return device_match_fwnode(dev, data);
+}
+
 static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
 {
        struct gpio_sim_chip *chip;
@@ -387,6 +406,10 @@ static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
        if (!chip)
                return -ENOMEM;
 
+       chip->request_map = devm_bitmap_zalloc(dev, num_lines, GFP_KERNEL);
+       if (!chip->request_map)
+               return -ENOMEM;
+
        chip->direction_map = devm_bitmap_alloc(dev, num_lines, GFP_KERNEL);
        if (!chip->direction_map)
                return -ENOMEM;
@@ -432,6 +455,7 @@ static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
        gc->get_direction = gpio_sim_get_direction;
        gc->set_config = gpio_sim_set_config;
        gc->to_irq = gpio_sim_to_irq;
+       gc->request = gpio_sim_request;
        gc->free = gpio_sim_free;
        gc->can_sleep = true;
 
@@ -439,8 +463,16 @@ static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
        if (ret)
                return ret;
 
-       /* Used by sysfs and configfs callbacks. */
-       dev_set_drvdata(&gc->gpiodev->dev, chip);
+       chip->dev = device_find_child(dev, swnode, gpio_sim_dev_match_fwnode);
+       if (!chip->dev)
+               return -ENODEV;
+
+       ret = devm_add_action_or_reset(dev, gpio_sim_put_device, chip->dev);
+       if (ret)
+               return ret;
+
+       /* Used by sysfs callbacks. */
+       dev_set_drvdata(chip->dev, chip);
 
        return gpio_sim_setup_sysfs(chip);
 }