[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230905082413.21954-1-brgl@bgdev.pl>
Date: Tue, 5 Sep 2023 10:24:13 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Linus Walleij <linus.walleij@...aro.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Kent Gibson <warthog618@...il.com>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: [PATCH v3] gpio: sim: don't fiddle with GPIOLIB private members
From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
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@...aro.org>
---
v1 -> v2:
- use get_dev_from_fwnode() instead of dereferencing fwnode directly
v2 -> v3:
- don't use fwnode internal fields, instead: iterate over the platform
device's children and locate the GPIO device
drivers/gpio/gpio-sim.c | 66 ++++++++++++++++++++++++++++++-----------
1 file changed, 48 insertions(+), 18 deletions(-)
diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index 271db3639a78..16a865e79559 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -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,9 @@ static DEFINE_IDA(gpio_sim_ida);
struct gpio_sim_chip {
struct gpio_chip gc;
+ struct fwnode_handle *swnode;
+ struct device *dev;
+ unsigned long *request_map;
unsigned long *direction_map;
unsigned long *value_map;
unsigned long *pull_map;
@@ -63,16 +65,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 +96,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:
@@ -181,7 +178,7 @@ static int gpio_sim_get_direction(struct gpio_chip *gc, unsigned int offset)
}
static int gpio_sim_set_config(struct gpio_chip *gc,
- unsigned int offset, unsigned long config)
+ unsigned int offset, unsigned long config)
{
struct gpio_sim_chip *chip = gpiochip_get_data(gc);
@@ -204,13 +201,25 @@ static int gpio_sim_to_irq(struct gpio_chip *gc, unsigned int offset)
return irq_create_mapping(chip->irq_sim, 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)
+ 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,
@@ -295,7 +304,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 +361,25 @@ 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_chip_set_device(struct device *dev, void *data)
+{
+ struct gpio_sim_chip *chip = data;
+
+ if (chip->swnode == dev->fwnode) {
+ chip->dev = dev;
+ return 1;
+ }
+
+ return 0;
+}
+
static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
{
struct gpio_sim_chip *chip;
@@ -387,6 +407,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 +456,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 +464,13 @@ 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->swnode = swnode;
+ ret = device_for_each_child(dev, chip, gpio_sim_chip_set_device);
+ if (!ret)
+ return -ENODEV;
+
+ /* Used by sysfs callbacks. */
+ dev_set_drvdata(chip->dev, chip);
return gpio_sim_setup_sysfs(chip);
}
--
2.39.2
Powered by blists - more mailing lists