[PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Bartosz Golaszewski]

This is another shot at the gpio-sim testing module. As there was no
reasoning with configfs maintainers for many months, this time the whole
concept of committable items has been dropped. Instead, each configfs
chip item (or rather a group - more on that later) exposes a new
attribute called 'live'. Writing 1 to it brings the chip on-line
(registers the platform device) and writing 0 tears it down.

There are some caveats to that approach - for example: we can't block
the user-space from deleting chip items when chips are live but is just
handled by silently destroying the chip device in the background.

Andy (rightfully) pointed out that parsing of the lists of line names is
awkward so in this iteration it's been replaced by a system that is more
elegant and will allow to easily extend configuration options for
specific GPIO lines. This is achieved by turning the chip's configfs
item into a configfs group and allowing the user-space to create
additional items inside it. The items must be called line<offset> (e.g.
line0, line12 etc.) where the offset part indicates to the module the
offset for which given item stores the configuration for. Within each
such line item, there are additional attributes that allow specifying
configuration for specific lines. Currently we only support the 'name'
attribute but I plan to extend that to support GPIO hogging too.

v1 -> v2:
- add selftests for gpio-sim
- add helper programs for selftests
- update the configfs rename callback to work with the new API introduced in
  v5.11
- fix a missing quote in the documentation
- use !! whenever using bits operation that are required to return 0 or 1
- use provided bitmap API instead of reimplementing copy or fill operations
- fix a deadlock in gpio_sim_direction_output()
- add new read-only configfs attributes for mapping of configfs items to GPIO
  device names
- and address other minor issues pointed out in reviews of v1

v2 -> v3:
- use devm_bitmap_alloc() instead of the zalloc variant if we're initializing
  the bitmap with 1s
- drop the patch exporting device_is_bound()
- don't return -ENODEV from dev_nam and chip_name configfs attributes, return
  a string indicating that the device is not available yet ('n/a')
- fix indentation where it makes sense
- don't protect IDA functions which use their own locking and where it's not
  needed
- use kmemdup() instead of kzalloc() + memcpy()
- collected review tags
- minor coding style fixes

v3 -> v4:
- return 'none' instead of 'n/a' from dev_name and chip_name before the device
  is registered
- use sysfs_emit() instead of s*printf()
- drop GPIO_SIM_MAX_PROP as it's only used in an array's definition where it's
  fine to hardcode the value

v4 -> v5:
- drop lib patches that are already upstream
- use BIT() instead of (1UL << bit) for flags
- fix refcounting for the configfs_dirent in rename()
- drop d_move() from the rename() callback
- free memory allocated for the live and pending groups in configfs_d_iput()
  and not in detach_groups()
- make sure that if a group of some name is in the live directory, a new group
  with the same name cannot be created in the pending directory

v5 -> v6:
- go back to using (1UL << bit) instead of BIT()
- if the live group dentry doesn't exist for whatever reason at the time when
  mkdir() in the pending group is called (would be a BUG()), return -ENOENT
  instead of -EEXIST which should only be returned if given subsystem already
  exists in either live or pending group

v6 -> v7:
- as detailed by Andy in commit 6fda593f3082 ("gpio: mockup: Convert to use
  software nodes") removing device properties after the platform device is
  removed but before the GPIO device gets dropped can lead to a use-after-free
  bug - use software nodes to manually control the freeing of the properties

v7 -> v8:
- fixed some minor coding style issues as pointed out by Andy

v8 -> v9:
- dropped the patches implementing committable-items and reworked the
  driver to not use them
- reworked the gpio-line-names property and configuring specific lines
  in general
- many smaller tweaks here and there

Bartosz Golaszewski (4):
  gpio: sim: new testing module
  selftests: gpio: provide a helper for reading chip info
  selftests: gpio: add a helper for reading GPIO line names
  selftests: gpio: add test cases for gpio-sim

 Documentation/admin-guide/gpio/gpio-sim.rst   |  67 ++
 drivers/gpio/Kconfig                          |   8 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-sim.c                       | 990 ++++++++++++++++++
 tools/testing/selftests/gpio/.gitignore       |   2 +
 tools/testing/selftests/gpio/Makefile         |   4 +-
 tools/testing/selftests/gpio/config           |   1 +
 tools/testing/selftests/gpio/gpio-chip-info.c |  57 +
 tools/testing/selftests/gpio/gpio-line-name.c |  55 +
 tools/testing/selftests/gpio/gpio-sim.sh      | 266 +++++
 10 files changed, 1449 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst
 create mode 100644 drivers/gpio/gpio-sim.c
 create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c
 create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c
 create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh

-- 
2.25.1

[PATCH v9 1/4] gpio: sim: new testing module

[Bartosz Golaszewski]

Implement a new, modern GPIO testing module controlled by configfs
attributes instead of module parameters. The goal of this driver is
to provide a replacement for gpio-mockup that will be easily extensible
with new features and doesn't require reloading the module to change
the setup.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 Documentation/admin-guide/gpio/gpio-sim.rst |  67 ++
 drivers/gpio/Kconfig                        |   8 +
 drivers/gpio/Makefile                       |   1 +
 drivers/gpio/gpio-sim.c                     | 990 ++++++++++++++++++++
 4 files changed, 1066 insertions(+)
 create mode 100644 Documentation/admin-guide/gpio/gpio-sim.rst
 create mode 100644 drivers/gpio/gpio-sim.c

diff --git a/Documentation/admin-guide/gpio/gpio-sim.rst b/Documentation/admin-guide/gpio/gpio-sim.rst
new file mode 100644
index 000000000000..7f2080f520d6
--- /dev/null
+++ b/Documentation/admin-guide/gpio/gpio-sim.rst
@@ -0,0 +1,67 @@
+.. SPDX-License-Identifier: GPL-2.0-or-later
+
+Configfs GPIO Simulator
+=======================
+
+The configfs GPIO Simulator (gpio-sim) provides a way to create simulated GPIO
+chips for testing purposes. The lines exposed by these chips can be accessed
+using the standard GPIO character device interface as well as manipulated
+using sysfs attributes.
+
+Creating simulated chips
+------------------------
+
+The gpio-sim module registers a configfs subsystem called 'gpio-sim'.
+
+In order to instantiate a new simulated chip, the user needs to mkdir() a new
+directory gpio-sim/. Inside each new directory, there's a set of attributes
+that can be used to configure the new chip. Additionally the user can mkdir()
+subdirectories inside the chip's directory that allow to pass additional
+configuration for specific lines. The name of those subdirectories must take
+the form of: 'line<offset>' (e.g. 'line0', 'line20', etc.) as the name will be
+used by the module to assign the config to the specific line at given offset.
+
+Once the confiuration is complete, the 'active' attribute must be set to 1 in
+order to instantiate the chip. It can be set back to 0 to destroy the simulated
+chip.
+
+Currently supported chip configuration attributes are:
+
+  num_lines - an unsigned integer value defining the number of GPIO lines to
+              export
+
+  label - a string defining the label for the GPIO chip
+
+Additionally two read-only attributes named 'chip_name' and 'dev_name' are
+exposed in order to provide users with a mapping from configfs directories to
+the actual devices created in the kernel. The former returns the name of the
+GPIO device as assigned by gpiolib (i.e. "gpiochip0", "gpiochip1", etc.). The
+latter returns the parent device name as defined by the gpio-sim driver (i.e.
+"gpio-sim.0", "gpio-sim.1", etc.). This allows user-space to map the configfs
+items both to the correct character device file as well as the associated entry
+in sysfs.
+
+Supported line configuration attributes are:
+
+  name - a string defining the name of this line as used by the
+         "gpio-line-names" device property
+
+Simulated GPIO chips can also be defined in device-tree. The compatible string
+must be: "gpio-simulator". Supported properties are:
+
+  "gpio-sim,label" - chip label
+
+Other standard GPIO properties (like "gpio-line-names", "ngpios" or gpio-hog)
+are also supported.
+
+Manipulating simulated lines
+----------------------------
+
+Each simulated GPIO chip creates a sysfs attribute group under its device
+directory called 'line-ctrl'. Inside each group, there's a separate attribute
+for each GPIO line. The name of the attribute is of the form 'gpioX' where X
+is the line's offset in the chip.
+
+Reading from a line attribute returns the current value. Writing to it (0 or 1)
+changes the configuration of the simulated pull-up/pull-down resistor
+(1 - pull-up, 0 - pull-down).
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 072ed610f9c6..5c3b29bddc5b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1693,6 +1693,14 @@ config GPIO_VIRTIO
 	  These virtual GPIOs can be routed to real GPIOs or attached to
 	  simulators on the host (like QEMU).
 
+config GPIO_SIM
+	tristate "GPIO Simulator Module"
+	select IRQ_SIM
+	select CONFIGFS_FS
+	help
+	  This enables the GPIO simulator - a configfs-based GPIO testing
+	  driver.
+
 endmenu
 
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 71ee9fc2ff83..f21577de2474 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -133,6 +133,7 @@ obj-$(CONFIG_GPIO_SAMA5D2_PIOBU)	+= gpio-sama5d2-piobu.o
 obj-$(CONFIG_GPIO_SCH311X)		+= gpio-sch311x.o
 obj-$(CONFIG_GPIO_SCH)			+= gpio-sch.o
 obj-$(CONFIG_GPIO_SIFIVE)		+= gpio-sifive.o
+obj-$(CONFIG_GPIO_SIM)			+= gpio-sim.o
 obj-$(CONFIG_GPIO_SIOX)			+= gpio-siox.o
 obj-$(CONFIG_GPIO_SL28CPLD)		+= gpio-sl28cpld.o
 obj-$(CONFIG_GPIO_SODAVILLE)		+= gpio-sodaville.o
diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
new file mode 100644
index 000000000000..2a1c548548a0
--- /dev/null
+++ b/drivers/gpio/gpio-sim.c
@@ -0,0 +1,990 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO testing driver based on configfs.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski <brgl@bgdev.pl>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/bitmap.h>
+#include <linux/configfs.h>
+#include <linux/device.h>
+#include <linux/gpio/driver.h>
+#include <linux/idr.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irq_sim.h>
+#include <linux/list.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/string_helpers.h>
+#include <linux/sysfs.h>
+
+#include "gpiolib.h"
+
+static DEFINE_IDA(gpio_sim_ida);
+
+struct gpio_sim_chip {
+	struct gpio_chip gc;
+	unsigned long *directions;
+	unsigned long *values;
+	unsigned long *pulls;
+	struct irq_domain *irq_sim;
+	struct mutex lock;
+	struct attribute_group attr_group;
+};
+
+struct gpio_sim_attribute {
+	struct device_attribute dev_attr;
+	unsigned int offset;
+};
+
+static struct gpio_sim_attribute *
+to_gpio_sim_attr(struct device_attribute *dev_attr)
+{
+	return container_of(dev_attr, struct gpio_sim_attribute, dev_attr);
+}
+
+static int gpio_sim_apply_pull(struct gpio_sim_chip *chip,
+			       unsigned int offset, int value)
+{
+	int curr_val, irq, irq_type, ret;
+	struct gpio_desc *desc;
+	struct gpio_chip *gc;
+
+	gc = &chip->gc;
+	desc = &gc->gpiodev->descs[offset];
+
+	mutex_lock(&chip->lock);
+
+	if (test_bit(FLAG_REQUESTED, &desc->flags) &&
+	    !test_bit(FLAG_IS_OUT, &desc->flags)) {
+		curr_val = !!test_bit(offset, chip->values);
+		if (curr_val == value)
+			goto set_pull;
+
+		/*
+		 * This is fine - it just means, nobody is listening
+		 * for interrupts on this line, otherwise
+		 * irq_create_mapping() would have been called from
+		 * the to_irq() callback.
+		 */
+		irq = irq_find_mapping(chip->irq_sim, offset);
+		if (!irq)
+			goto set_value;
+
+		irq_type = irq_get_trigger_type(irq);
+
+		if ((value && (irq_type & IRQ_TYPE_EDGE_RISING)) ||
+		    (!value && (irq_type & IRQ_TYPE_EDGE_FALLING))) {
+			ret = irq_set_irqchip_state(irq, IRQCHIP_STATE_PENDING,
+						    true);
+			if (ret)
+				goto set_pull;
+		}
+	}
+
+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))
+		__assign_bit(offset, chip->values, value);
+
+set_pull:
+	__assign_bit(offset, chip->pulls, value);
+	mutex_unlock(&chip->lock);
+	return 0;
+}
+
+static int gpio_sim_get(struct gpio_chip *gc, unsigned int offset)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+	int ret;
+
+	mutex_lock(&chip->lock);
+	ret = !!test_bit(offset, chip->values);
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+static void gpio_sim_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	mutex_lock(&chip->lock);
+	__assign_bit(offset, chip->values, value);
+	mutex_unlock(&chip->lock);
+}
+
+static int gpio_sim_get_multiple(struct gpio_chip *gc,
+				 unsigned long *mask, unsigned long *bits)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	mutex_lock(&chip->lock);
+	bitmap_copy(bits, chip->values, gc->ngpio);
+	mutex_unlock(&chip->lock);
+
+	return 0;
+}
+
+static void gpio_sim_set_multiple(struct gpio_chip *gc,
+				  unsigned long *mask, unsigned long *bits)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	mutex_lock(&chip->lock);
+	bitmap_copy(chip->values, bits, gc->ngpio);
+	mutex_unlock(&chip->lock);
+}
+
+static int gpio_sim_direction_output(struct gpio_chip *gc,
+				     unsigned int offset, int value)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	mutex_lock(&chip->lock);
+	__clear_bit(offset, chip->directions);
+	__assign_bit(offset, chip->values, value);
+	mutex_unlock(&chip->lock);
+
+	return 0;
+}
+
+static int gpio_sim_direction_input(struct gpio_chip *gc, unsigned int offset)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	mutex_lock(&chip->lock);
+	__set_bit(offset, chip->directions);
+	mutex_unlock(&chip->lock);
+
+	return 0;
+}
+
+static int gpio_sim_get_direction(struct gpio_chip *gc, unsigned int offset)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+	int direction;
+
+	mutex_lock(&chip->lock);
+	direction = !!test_bit(offset, chip->directions);
+	mutex_unlock(&chip->lock);
+
+	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)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	switch (pinconf_to_config_param(config)) {
+	case PIN_CONFIG_BIAS_PULL_UP:
+		return gpio_sim_apply_pull(chip, offset, 1);
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		return gpio_sim_apply_pull(chip, offset, 0);
+	default:
+		break;
+	}
+
+	return -ENOTSUPP;
+}
+
+static int gpio_sim_to_irq(struct gpio_chip *gc, unsigned int offset)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	return irq_create_mapping(chip->irq_sim, offset);
+}
+
+static void gpio_sim_free(struct gpio_chip *gc, unsigned int offset)
+{
+	struct gpio_sim_chip *chip = gpiochip_get_data(gc);
+
+	mutex_lock(&chip->lock);
+	__assign_bit(offset, chip->values, !!test_bit(offset, chip->pulls));
+	mutex_unlock(&chip->lock);
+}
+
+static ssize_t gpio_sim_sysfs_line_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr);
+	struct gpio_sim_chip *chip = dev_get_drvdata(dev);
+	int ret;
+
+	mutex_lock(&chip->lock);
+	ret = sysfs_emit(buf, "%u\n",
+			 !!test_bit(line_attr->offset, chip->values));
+	mutex_unlock(&chip->lock);
+
+	return ret;
+}
+
+static ssize_t gpio_sim_sysfs_line_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t len)
+{
+	struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr);
+	struct gpio_sim_chip *chip = dev_get_drvdata(dev);
+	int ret, val;
+
+	if (len > 2 || (buf[0] != '0' && buf[0] != '1'))
+		return -EINVAL;
+
+	val = buf[0] == '0' ? 0 : 1;
+
+	ret = gpio_sim_apply_pull(chip, line_attr->offset, val);
+	if (ret)
+		return ret;
+
+	return len;
+}
+
+static void gpio_sim_mutex_destroy(void *data)
+{
+	struct mutex *lock = data;
+
+	mutex_destroy(lock);
+}
+
+static void gpio_sim_sysfs_remove(void *data)
+{
+	struct gpio_sim_chip *chip = data;
+
+	sysfs_remove_group(&chip->gc.parent->kobj, &chip->attr_group);
+}
+
+static int gpio_sim_setup_sysfs(struct gpio_sim_chip *chip)
+{
+	unsigned int i, num_lines = chip->gc.ngpio;
+	struct device *dev = chip->gc.parent;
+	struct gpio_sim_attribute *line_attr;
+	struct device_attribute *dev_attr;
+	struct attribute **attrs;
+	int ret;
+
+	attrs = devm_kcalloc(dev, sizeof(*attrs), num_lines + 1, GFP_KERNEL);
+	if (!attrs)
+		return -ENOMEM;
+
+	for (i = 0; i < num_lines; i++) {
+		line_attr = devm_kzalloc(dev, sizeof(*line_attr), GFP_KERNEL);
+		if (!line_attr)
+			return -ENOMEM;
+
+		line_attr->offset = i;
+
+		dev_attr = &line_attr->dev_attr;
+		sysfs_attr_init(&dev_attr->attr);
+
+		dev_attr->attr.name = devm_kasprintf(dev, GFP_KERNEL,
+						     "gpio%u", i);
+		if (!dev_attr->attr.name)
+			return -ENOMEM;
+
+		dev_attr->attr.mode = 0644;
+
+		dev_attr->show = gpio_sim_sysfs_line_show;
+		dev_attr->store = gpio_sim_sysfs_line_store;
+
+		attrs[i] = &dev_attr->attr;
+	}
+
+	chip->attr_group.name = "control";
+	chip->attr_group.attrs = attrs;
+
+	ret = sysfs_create_group(&dev->kobj, &chip->attr_group);
+	if (ret)
+		return ret;
+
+	return devm_add_action_or_reset(dev, gpio_sim_sysfs_remove, chip);
+}
+
+static int gpio_sim_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct gpio_sim_chip *chip;
+	struct gpio_chip *gc;
+	const char *label;
+	u32 num_lines;
+	int ret;
+
+	ret = device_property_read_u32(dev, "ngpios", &num_lines);
+	if (ret)
+		return ret;
+
+	ret = device_property_read_string(dev, "gpio-sim,label", &label);
+	if (ret)
+		label = dev_name(dev);
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->directions = devm_bitmap_alloc(dev, num_lines, GFP_KERNEL);
+	if (!chip->directions)
+		return -ENOMEM;
+
+	/* Default to input mode. */
+	bitmap_fill(chip->directions, num_lines);
+
+	chip->values = devm_bitmap_zalloc(dev, num_lines, GFP_KERNEL);
+	if (!chip->values)
+		return -ENOMEM;
+
+	chip->pulls = devm_bitmap_zalloc(dev, num_lines, GFP_KERNEL);
+	if (!chip->pulls)
+		return -ENOMEM;
+
+	chip->irq_sim = devm_irq_domain_create_sim(dev, NULL, num_lines);
+	if (IS_ERR(chip->irq_sim))
+		return PTR_ERR(chip->irq_sim);
+
+	mutex_init(&chip->lock);
+	ret = devm_add_action_or_reset(dev, gpio_sim_mutex_destroy,
+				       &chip->lock);
+	if (ret)
+		return ret;
+
+	gc = &chip->gc;
+	gc->base = -1;
+	gc->ngpio = num_lines;
+	gc->label = label;
+	gc->owner = THIS_MODULE;
+	gc->parent = dev;
+	gc->get = gpio_sim_get;
+	gc->set = gpio_sim_set;
+	gc->get_multiple = gpio_sim_get_multiple;
+	gc->set_multiple = gpio_sim_set_multiple;
+	gc->direction_output = gpio_sim_direction_output;
+	gc->direction_input = gpio_sim_direction_input;
+	gc->get_direction = gpio_sim_get_direction;
+	gc->set_config = gpio_sim_set_config;
+	gc->to_irq = gpio_sim_to_irq;
+	gc->free = gpio_sim_free;
+
+	ret = devm_gpiochip_add_data(dev, gc, chip);
+	if (ret)
+		return ret;
+
+	/* Used by sysfs and configfs callbacks. */
+	dev_set_drvdata(dev, chip);
+
+	return gpio_sim_setup_sysfs(chip);
+}
+
+static const struct of_device_id gpio_sim_of_match[] = {
+	{ .compatible = "gpio-simulator" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, gpio_sim_of_match);
+
+static struct platform_driver gpio_sim_driver = {
+	.driver = {
+		.name = "gpio-sim",
+		.of_match_table = gpio_sim_of_match,
+	},
+	.probe = gpio_sim_probe,
+};
+
+struct gpio_sim_chip_ctx {
+	struct config_group group;
+
+	/*
+	 * If pdev is NULL, the item is 'pending' (waiting for configuration).
+	 * Once the pointer is assigned, the device has been created and the
+	 * item is 'live'.
+	 */
+	struct platform_device *pdev;
+	int id;
+
+	/*
+	 * Each configfs filesystem operation is protected with the subsystem
+	 * mutex. Each separate attribute is protected with the buffer mutex.
+	 * This structure however can be modified by callbacks of different
+	 * attributes so we need another lock.
+	 */
+	struct mutex lock;
+
+	char label[32];
+	unsigned int num_lines;
+
+	struct list_head line_ctx_list;
+};
+
+struct gpio_sim_line_ctx {
+	struct config_item item;
+	struct list_head list;
+
+	/*
+	 * We could have used the ci_parent field of the config_item but
+	 * configfs is stupid and calls the item's release callback after
+	 * already having cleared the parent pointer even though the parent
+	 * is guaranteed to survive the child...
+	 *
+	 * So we need to store the pointer to the parent struct here.
+	 */
+	struct gpio_sim_chip_ctx *parent;
+
+	/* Same as the chip context. */
+	struct mutex lock;
+
+	unsigned int offset;
+	char *name;
+};
+
+static struct gpio_sim_chip_ctx *to_gpio_sim_chip_ctx(struct config_item *item)
+{
+	struct config_group *group = container_of(item, struct config_group,
+						  cg_item);
+
+	return container_of(group, struct gpio_sim_chip_ctx, group);
+}
+
+static struct gpio_sim_line_ctx *to_gpio_sim_line_ctx(struct config_item *item)
+{
+	return container_of(item, struct gpio_sim_line_ctx, item);
+}
+
+static bool gpio_sim_chip_live(struct gpio_sim_chip_ctx *ctx)
+{
+	return !!ctx->pdev;
+}
+
+static char *gpio_sim_strdup_trimmed(const char *str, size_t count)
+{
+	char *dup, *trimmed, *ret;
+
+	dup = kstrndup(str, count, GFP_KERNEL);
+	if (!dup)
+		return NULL;
+
+	trimmed = strstrip(dup);
+	ret = kstrdup(trimmed, GFP_KERNEL);
+	kfree(dup);
+	return ret;
+}
+
+static ssize_t gpio_sim_config_chip_dev_name_show(struct config_item *item,
+						  char *page)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	struct platform_device *pdev;
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	pdev = ctx->pdev;
+	if (pdev)
+		ret = sprintf(page, "%s\n", dev_name(&pdev->dev));
+	else
+		ret = sprintf(page, "gpio-sim.%d\n", ctx->id);
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}
+
+CONFIGFS_ATTR_RO(gpio_sim_config_chip_, dev_name);
+
+static ssize_t gpio_sim_config_chip_chip_name_show(struct config_item *item,
+						   char *page)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	struct gpio_sim_chip *chip = NULL;
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	if (gpio_sim_chip_live(ctx))
+		chip = dev_get_drvdata(&ctx->pdev->dev);
+
+	if (chip)
+		ret = sprintf(page, "%s\n", dev_name(&chip->gc.gpiodev->dev));
+	else
+		ret = sprintf(page, "none\n");
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}
+
+CONFIGFS_ATTR_RO(gpio_sim_config_chip_, chip_name);
+
+static ssize_t
+gpio_sim_config_chip_label_show(struct config_item *item, char *page)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	ret = sprintf(page, "%s\n", ctx->label);
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}
+
+static ssize_t gpio_sim_config_chip_label_store(struct config_item *item,
+						const char *page, size_t count)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	char *trimmed;
+	int ret;
+
+	mutex_lock(&ctx->lock);
+
+	if (gpio_sim_chip_live(ctx)) {
+		mutex_unlock(&ctx->lock);
+		return -EBUSY;
+	}
+
+	trimmed = gpio_sim_strdup_trimmed(page, count);
+	if (!trimmed) {
+		mutex_unlock(&ctx->lock);
+		return -ENOMEM;
+	}
+
+	ret = snprintf(ctx->label, sizeof(ctx->label), "%s", trimmed);
+	kfree(trimmed);
+	if (ret < 0) {
+		mutex_unlock(&ctx->lock);
+		return ret;
+	}
+
+	mutex_unlock(&ctx->lock);
+	return count;
+}
+
+CONFIGFS_ATTR(gpio_sim_config_chip_, label);
+
+static ssize_t
+gpio_sim_config_chip_num_lines_show(struct config_item *item, char *page)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	ret = sprintf(page, "%u\n", ctx->num_lines);
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}
+
+static ssize_t
+gpio_sim_config_chip_num_lines_store(struct config_item *item,
+				     const char *page, size_t count)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	unsigned int num_lines;
+	int ret;
+
+	mutex_lock(&ctx->lock);
+
+	if (gpio_sim_chip_live(ctx)) {
+		mutex_unlock(&ctx->lock);
+		return -EBUSY;
+	}
+
+	ret = kstrtouint(page, 10, &num_lines);
+	if (ret) {
+		mutex_unlock(&ctx->lock);
+		return ret;
+	}
+
+	if (num_lines == 0) {
+		mutex_unlock(&ctx->lock);
+		return -EINVAL;
+	}
+
+	ctx->num_lines = num_lines;
+
+	mutex_unlock(&ctx->lock);
+	return count;
+}
+
+CONFIGFS_ATTR(gpio_sim_config_chip_, num_lines);
+
+static ssize_t
+gpio_sim_config_chip_live_show(struct config_item *item, char *page)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	ret = sprintf(page, "%c\n", gpio_sim_chip_live(ctx) ? '1' : '0');
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}
+
+static char **gpio_sim_make_line_names(struct gpio_sim_chip_ctx *chip_ctx,
+				       unsigned int *line_names_size)
+{
+	struct gpio_sim_line_ctx *line_ctx;
+	unsigned int max_offset = 0;
+	bool has_line_names = false;
+	char **line_names;
+
+	list_for_each_entry(line_ctx, &chip_ctx->line_ctx_list, list) {
+		if (line_ctx->name) {
+			if (line_ctx->offset > max_offset)
+				max_offset = line_ctx->offset;
+
+			/*
+			 * max_offset can stay at 0 so it's not an indicator
+			 * of whether line names were configured at all.
+			 */
+			has_line_names = true;
+		}
+	}
+
+	if (!has_line_names)
+		/*
+		 * This is not an error - NULL means, there are no line
+		 * names configured.
+		 */
+		return NULL;
+
+	*line_names_size = max_offset + 1;
+
+	line_names = kcalloc(*line_names_size, sizeof(*line_names), GFP_KERNEL);
+	if (!line_names)
+		return ERR_PTR(-ENOMEM);
+
+	list_for_each_entry(line_ctx, &chip_ctx->line_ctx_list, list) {
+		if (line_ctx->name)
+			line_names[line_ctx->offset] = line_ctx->name;
+	}
+
+	return line_names;
+}
+
+static int gpio_sim_activate_chip_unlocked(struct gpio_sim_chip_ctx *ctx)
+{
+	unsigned int prop_idx = 0, line_names_size = 0;
+	struct platform_device_info pdevinfo;
+	struct property_entry properties[4]; /* Max 3 properties + sentinel. */
+	struct fwnode_handle *fwnode;
+	struct platform_device *pdev;
+	char **line_names;
+
+	memset(&pdevinfo, 0, sizeof(pdevinfo));
+	memset(properties, 0, sizeof(properties));
+
+	properties[prop_idx++] = PROPERTY_ENTRY_U32("ngpios",
+						    ctx->num_lines);
+
+	if (ctx->label[0] != '\0')
+		properties[prop_idx++] = PROPERTY_ENTRY_STRING("gpio-sim,label",
+							       ctx->label);
+
+	line_names = gpio_sim_make_line_names(ctx, &line_names_size);
+	if (IS_ERR(line_names))
+		return PTR_ERR(line_names);
+
+	if (line_names)
+		properties[prop_idx++] = PROPERTY_ENTRY_STRING_ARRAY_LEN(
+						"gpio-line-names",
+						line_names, line_names_size);
+
+	fwnode = fwnode_create_software_node(properties, NULL);
+	/*
+	 * fwnode_create_software_node() makes a deep copy of the properties,
+	 * so no need to store the array of line names.
+	 */
+	kfree_strarray(line_names, line_names_size);
+	if (IS_ERR(fwnode))
+		return PTR_ERR(fwnode);
+
+	pdevinfo.name = "gpio-sim";
+	pdevinfo.fwnode = fwnode;
+	pdevinfo.id = ctx->id;
+
+	pdev = platform_device_register_full(&pdevinfo);
+	if (IS_ERR(pdev)) {
+		fwnode_remove_software_node(fwnode);
+		return PTR_ERR(pdev);
+	}
+
+	ctx->pdev = pdev;
+
+	return 0;
+}
+
+static void gpio_sim_deactivate_chip_unlocked(struct gpio_sim_chip_ctx *ctx)
+{
+	struct fwnode_handle *fwnode;
+
+	fwnode = dev_fwnode(&ctx->pdev->dev);
+	platform_device_unregister(ctx->pdev);
+	fwnode_remove_software_node(fwnode);
+	ctx->pdev = NULL;
+}
+
+static ssize_t
+gpio_sim_config_chip_live_store(struct config_item *item,
+				  const char *page, size_t count)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+	int live, ret;
+
+	ret = kstrtouint(page, 10, &live);
+	if (ret)
+		return ret;
+
+	mutex_lock(&ctx->lock);
+
+	if ((live == 0 && !gpio_sim_chip_live(ctx)) ||
+	    (live == 1 && gpio_sim_chip_live(ctx)))
+		ret = -EPERM;
+	else if (live == 1)
+		ret = gpio_sim_activate_chip_unlocked(ctx);
+	else if (live == 0)
+		gpio_sim_deactivate_chip_unlocked(ctx);
+	else
+		ret = -EINVAL;
+
+	mutex_unlock(&ctx->lock);
+
+	return ret ?: count;
+}
+
+CONFIGFS_ATTR(gpio_sim_config_chip_, live);
+
+static struct configfs_attribute *gpio_sim_config_chip_attrs[] = {
+	&gpio_sim_config_chip_attr_dev_name,
+	&gpio_sim_config_chip_attr_chip_name,
+	&gpio_sim_config_chip_attr_label,
+	&gpio_sim_config_chip_attr_num_lines,
+	&gpio_sim_config_chip_attr_live,
+	NULL
+};
+
+static ssize_t
+gpio_sim_config_line_name_show(struct config_item *item, char *page)
+{
+	struct gpio_sim_line_ctx *ctx = to_gpio_sim_line_ctx(item);
+	int ret;
+
+	mutex_lock(&ctx->lock);
+	ret = sprintf(page, "%s\n", ctx->name ?: "");
+	mutex_unlock(&ctx->lock);
+
+	return ret;
+}
+
+static ssize_t gpio_sim_config_line_name_store(struct config_item *item,
+					       const char *page, size_t count)
+{
+	struct gpio_sim_line_ctx *line_ctx = to_gpio_sim_line_ctx(item);
+	struct gpio_sim_chip_ctx *chip_ctx = line_ctx->parent;
+	char *trimmed;
+
+	mutex_lock(&chip_ctx->lock);
+
+	if (gpio_sim_chip_live(chip_ctx)) {
+		mutex_unlock(&chip_ctx->lock);
+		return -EBUSY;
+	}
+
+	trimmed = gpio_sim_strdup_trimmed(page, count);
+	if (!trimmed) {
+		mutex_unlock(&chip_ctx->lock);
+		return -ENOMEM;
+	}
+
+	mutex_lock(&line_ctx->lock);
+
+	kfree(line_ctx->name);
+	line_ctx->name = trimmed;
+
+	mutex_unlock(&line_ctx->lock);
+	mutex_unlock(&chip_ctx->lock);
+
+	return count;
+}
+
+CONFIGFS_ATTR(gpio_sim_config_line_, name);
+
+static struct configfs_attribute *gpio_sim_line_config_attrs[] = {
+	&gpio_sim_config_line_attr_name,
+	NULL,
+};
+
+static void gpio_sim_line_item_release(struct config_item *item)
+{
+	struct gpio_sim_line_ctx *line_ctx = to_gpio_sim_line_ctx(item);
+	struct gpio_sim_chip_ctx *chip_ctx = line_ctx->parent;
+
+	mutex_lock(&chip_ctx->lock);
+	list_del(&line_ctx->list);
+	mutex_unlock(&chip_ctx->lock);
+
+	mutex_destroy(&line_ctx->lock);
+	kfree(line_ctx->name);
+	kfree(line_ctx);
+}
+
+static struct configfs_item_operations gpio_sim_config_line_item_ops = {
+	.release	= gpio_sim_line_item_release,
+};
+
+static const struct config_item_type gpio_sim_line_config_type = {
+	.ct_item_ops	= &gpio_sim_config_line_item_ops,
+	.ct_attrs	= gpio_sim_line_config_attrs,
+	.ct_owner       = THIS_MODULE,
+};
+
+static struct config_item *
+gpio_sim_config_make_line_item(struct config_group *group, const char *name)
+{
+	struct gpio_sim_chip_ctx *chip_ctx;
+	struct gpio_sim_line_ctx *line_ctx;
+	unsigned int offset;
+	int ret, nchar;
+
+	ret = sscanf(name, "line%u%n", &offset, &nchar);
+	if (ret != 1 || nchar != strlen(name))
+		return ERR_PTR(-EINVAL);
+
+	chip_ctx = to_gpio_sim_chip_ctx(&group->cg_item);
+
+	mutex_lock(&chip_ctx->lock);
+
+	if (gpio_sim_chip_live(chip_ctx)) {
+		mutex_unlock(&chip_ctx->lock);
+		return ERR_PTR(-EBUSY);
+	}
+
+	line_ctx = kzalloc(sizeof(*line_ctx), GFP_KERNEL);
+	if (!line_ctx) {
+		mutex_unlock(&chip_ctx->lock);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	config_item_init_type_name(&line_ctx->item, name,
+				   &gpio_sim_line_config_type);
+
+	line_ctx->parent = chip_ctx;
+	line_ctx->offset = offset;
+	list_add_tail(&line_ctx->list, &chip_ctx->line_ctx_list);
+	mutex_init(&line_ctx->lock);
+
+	mutex_unlock(&chip_ctx->lock);
+
+	return &line_ctx->item;
+}
+
+static void gpio_sim_chip_item_release(struct config_item *item)
+{
+	struct gpio_sim_chip_ctx *ctx = to_gpio_sim_chip_ctx(item);
+
+	if (gpio_sim_chip_live(ctx))
+		gpio_sim_deactivate_chip_unlocked(ctx);
+
+	mutex_destroy(&ctx->lock);
+	ida_free(&gpio_sim_ida, ctx->id);
+	kfree(ctx);
+}
+
+static struct configfs_item_operations gpio_sim_config_chip_item_ops = {
+	.release	= gpio_sim_chip_item_release,
+};
+
+static struct configfs_group_operations gpio_sim_config_chip_group_ops = {
+	.make_item	= gpio_sim_config_make_line_item,
+};
+
+static const struct config_item_type gpio_sim_chip_group_config_type = {
+	.ct_item_ops	= &gpio_sim_config_chip_item_ops,
+	.ct_group_ops	= &gpio_sim_config_chip_group_ops,
+	.ct_attrs	= gpio_sim_config_chip_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct config_group *
+gpio_sim_config_make_chip_group(struct config_group *group, const char *name)
+{
+	struct gpio_sim_chip_ctx *ctx;
+	int id;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	id = ida_alloc(&gpio_sim_ida, GFP_KERNEL);
+	if (id < 0) {
+		kfree(ctx);
+		return ERR_PTR(id);
+	}
+
+	config_group_init_type_name(&ctx->group, name,
+				    &gpio_sim_chip_group_config_type);
+	ctx->num_lines = 1;
+	ctx->id = id;
+	mutex_init(&ctx->lock);
+	INIT_LIST_HEAD(&ctx->line_ctx_list);
+
+	return &ctx->group;
+}
+
+static struct configfs_group_operations gpio_sim_config_group_ops = {
+	.make_group	= gpio_sim_config_make_chip_group,
+};
+
+static const struct config_item_type gpio_sim_config_type = {
+	.ct_group_ops	= &gpio_sim_config_group_ops,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem gpio_sim_config_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf	= "gpio-sim",
+			.ci_type	= &gpio_sim_config_type,
+		},
+	},
+};
+
+static int __init gpio_sim_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&gpio_sim_driver);
+	if (ret) {
+		pr_err("Error %d while registering the platform driver\n", ret);
+		return ret;
+	}
+
+	config_group_init(&gpio_sim_config_subsys.su_group);
+	mutex_init(&gpio_sim_config_subsys.su_mutex);
+	ret = configfs_register_subsystem(&gpio_sim_config_subsys);
+	if (ret) {
+		pr_err("Error %d while registering the configfs subsystem %s\n",
+		       ret, gpio_sim_config_subsys.su_group.cg_item.ci_namebuf);
+		mutex_destroy(&gpio_sim_config_subsys.su_mutex);
+		platform_driver_unregister(&gpio_sim_driver);
+		return ret;
+	}
+
+	return 0;
+}
+module_init(gpio_sim_init);
+
+static void __exit gpio_sim_exit(void)
+{
+	configfs_unregister_subsystem(&gpio_sim_config_subsys);
+	mutex_destroy(&gpio_sim_config_subsys.su_mutex);
+	platform_driver_unregister(&gpio_sim_driver);
+}
+module_exit(gpio_sim_exit);
+
+MODULE_AUTHOR("Bartosz Golaszewski <brgl@bgdev.pl");
+MODULE_DESCRIPTION("GPIO Simulator Module");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH v9 1/4] gpio: sim: new testing module

[Linus Walleij]

On Thu, Nov 18, 2021 at 3:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Implement a new, modern GPIO testing module controlled by configfs
> attributes instead of module parameters. The goal of this driver is
> to provide a replacement for gpio-mockup that will be easily extensible
> with new features and doesn't require reloading the module to change
> the setup.
>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>

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

Yours,
Linus Walleij

[PATCH v9 2/4] selftests: gpio: provide a helper for reading chip info

[Bartosz Golaszewski]

Add a simple program that allows to retrieve chip properties from the
GPIO character device. This will be used in gpio-sim selftests.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 tools/testing/selftests/gpio/.gitignore       |  1 +
 tools/testing/selftests/gpio/Makefile         |  2 +-
 tools/testing/selftests/gpio/gpio-chip-info.c | 57 +++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/gpio/gpio-chip-info.c

diff --git a/tools/testing/selftests/gpio/.gitignore b/tools/testing/selftests/gpio/.gitignore
index a4969f7ee020..4ea4f58dab1a 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 gpio-mockup-cdev
+gpio-chip-info
diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 39f2bbe8dd3d..84b48547f94c 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
 
 TEST_PROGS := gpio-mockup.sh
 TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
 
 include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-chip-info.c b/tools/testing/selftests/gpio/gpio-chip-info.c
new file mode 100644
index 000000000000..8f2d2e23e9f6
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-chip-info.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading chip information.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski <brgl@bgdev.pl>
+ */
+
+#include <fcntl.h>
+#include <linux/gpio.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+
+static void print_usage(void)
+{
+	printf("usage:\n");
+	printf("  gpio-chip-info <chip path> [name|label|num-lines]\n");
+}
+
+int main(int argc, char **argv)
+{
+	struct gpiochip_info info;
+	int fd, ret;
+
+	if (argc !=3) {
+		print_usage();
+		return EXIT_FAILURE;
+	}
+
+	fd = open(argv[1], O_RDWR);
+	if (fd < 0) {
+		perror("unable to open the GPIO chip");
+		return EXIT_FAILURE;
+	}
+
+	memset(&info, 0, sizeof(info));
+	ret = ioctl(fd, GPIO_GET_CHIPINFO_IOCTL, &info);
+	if (ret) {
+		perror("chip info ioctl failed");
+		return EXIT_FAILURE;
+	}
+
+	if (strcmp(argv[2], "name") == 0) {
+		printf("%s\n", info.name);
+	} else if (strcmp(argv[2], "label") == 0) {
+		printf("%s\n", info.label);
+	} else if (strcmp(argv[2], "num-lines") == 0) {
+		printf("%u\n", info.lines);
+	} else {
+		fprintf(stderr, "unknown command: %s\n", argv[2]);
+		return EXIT_FAILURE;
+	}
+
+	return EXIT_SUCCESS;
+}
-- 
2.25.1

[PATCH v9 3/4] selftests: gpio: add a helper for reading GPIO line names

[Bartosz Golaszewski]

Add a simple program that allows to read GPIO line names from the
character device. This will be used in gpio-sim selftests.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 tools/testing/selftests/gpio/.gitignore       |  1 +
 tools/testing/selftests/gpio/Makefile         |  2 +-
 tools/testing/selftests/gpio/gpio-line-name.c | 55 +++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/gpio/gpio-line-name.c

diff --git a/tools/testing/selftests/gpio/.gitignore b/tools/testing/selftests/gpio/.gitignore
index 4ea4f58dab1a..ededb077a3a6 100644
--- a/tools/testing/selftests/gpio/.gitignore
+++ b/tools/testing/selftests/gpio/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 gpio-mockup-cdev
 gpio-chip-info
+gpio-line-name
diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index 84b48547f94c..d7d8f1985d99 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -2,6 +2,6 @@
 
 TEST_PROGS := gpio-mockup.sh
 TEST_FILES := gpio-mockup-sysfs.sh
-TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info
+TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
 
 include ../lib.mk
diff --git a/tools/testing/selftests/gpio/gpio-line-name.c b/tools/testing/selftests/gpio/gpio-line-name.c
new file mode 100644
index 000000000000..e635cfadbded
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-line-name.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * GPIO character device helper for reading line names.
+ *
+ * Copyright (C) 2021 Bartosz Golaszewski <brgl@bgdev.pl>
+ */
+
+#include <fcntl.h>
+#include <linux/gpio.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+
+static void print_usage(void)
+{
+	printf("usage:\n");
+	printf("  gpio-line-name <chip path> <line offset>\n");
+}
+
+int main(int argc, char **argv)
+{
+	struct gpio_v2_line_info info;
+	int fd, ret;
+	char *endp;
+
+	if (argc != 3) {
+		print_usage();
+		return EXIT_FAILURE;
+	}
+
+	fd = open(argv[1], O_RDWR);
+	if (fd < 0) {
+		perror("unable to open the GPIO chip");
+		return EXIT_FAILURE;
+	}
+
+	memset(&info, 0, sizeof(info));
+	info.offset = strtoul(argv[2], &endp, 10);
+	if (*endp != '\0') {
+		print_usage();
+		return EXIT_FAILURE;
+	}
+
+	ret = ioctl(fd, GPIO_V2_GET_LINEINFO_IOCTL, &info);
+	if (ret) {
+		perror("line info ioctl failed");
+		return EXIT_FAILURE;
+	}
+
+	printf("%s\n", info.name);
+
+	return EXIT_SUCCESS;
+}
-- 
2.25.1

[PATCH v9 4/4] selftests: gpio: add test cases for gpio-sim

[Bartosz Golaszewski]

Add a set of tests for the new gpio-sim module. This is a pure shell
test-suite and uses the helper programs available in the gpio selftests
directory. These test-cases only test the functionalities exposed by the
gpio-sim driver, not those handled by core gpiolib code.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 tools/testing/selftests/gpio/Makefile    |   2 +-
 tools/testing/selftests/gpio/config      |   1 +
 tools/testing/selftests/gpio/gpio-sim.sh | 266 +++++++++++++++++++++++
 3 files changed, 268 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/gpio/gpio-sim.sh

diff --git a/tools/testing/selftests/gpio/Makefile b/tools/testing/selftests/gpio/Makefile
index d7d8f1985d99..4c6df61c76a8 100644
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
-TEST_PROGS := gpio-mockup.sh
+TEST_PROGS := gpio-mockup.sh gpio-sim.sh
 TEST_FILES := gpio-mockup-sysfs.sh
 TEST_GEN_PROGS_EXTENDED := gpio-mockup-cdev gpio-chip-info gpio-line-name
 
diff --git a/tools/testing/selftests/gpio/config b/tools/testing/selftests/gpio/config
index ce100342c20b..409a8532facc 100644
--- a/tools/testing/selftests/gpio/config
+++ b/tools/testing/selftests/gpio/config
@@ -1,3 +1,4 @@
 CONFIG_GPIOLIB=y
 CONFIG_GPIO_CDEV=y
 CONFIG_GPIO_MOCKUP=m
+CONFIG_GPIO_SIM=m
diff --git a/tools/testing/selftests/gpio/gpio-sim.sh b/tools/testing/selftests/gpio/gpio-sim.sh
new file mode 100755
index 000000000000..99ce808d772a
--- /dev/null
+++ b/tools/testing/selftests/gpio/gpio-sim.sh
@@ -0,0 +1,266 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2021 Bartosz Golaszewski <brgl@bgdev.pl>
+
+BASE_DIR=`dirname $0`
+CONFIGFS_DIR="/sys/kernel/config/gpio-sim"
+MODULE="gpio-sim"
+
+fail() {
+	echo "$*" >&2
+	echo "GPIO $MODULE test FAIL"
+	exit 1
+}
+
+skip() {
+	echo "$*" >&2
+	echo "GPIO $MODULE test SKIP"
+	exit 4
+}
+
+remove_chip() {
+	local CHIP=$1
+
+	LINES=`ls $CONFIGFS_DIR/$CHIP/ | egrep ^line`
+	if [ "$?" == 0 ]; then
+		for LINE in $LINES; do
+			rmdir $CONFIGFS_DIR/$CHIP/$LINE || fail "Unable to remove the line"
+		done
+	fi
+
+	rmdir $CONFIGFS_DIR/$CHIP || fail "Unable to remove the chip"
+}
+
+configfs_cleanup() {
+	for CHIP in `ls $CONFIGFS_DIR/`; do
+		remove_chip $CHIP
+	done
+}
+
+create_chip() {
+	local CHIP=$1
+	local CHIP_DIR="$CONFIGFS_DIR/$CHIP"
+
+	mkdir $CHIP_DIR
+}
+
+set_label() {
+	local CHIP=$1
+	local LABEL=$2
+
+	echo $LABEL > $CONFIGFS_DIR/$CHIP/label || fail "Unable to set the chip label"
+}
+
+set_num_lines() {
+	local CHIP=$1
+	local NUM_LINES=$2
+
+	echo $NUM_LINES > $CONFIGFS_DIR/$CHIP/num_lines || fail "Unable to set the number of lines"
+}
+
+set_line_name() {
+	local CHIP=$1
+	local OFFSET=$2
+	local NAME=$3
+	local LINE_DIR=$CONFIGFS_DIR/$CHIP/line$OFFSET
+
+	test -d $LINE_DIR || mkdir $LINE_DIR
+	echo $NAME > $LINE_DIR/name || fail "Unable to set the line name"
+}
+
+enable_chip() {
+	local CHIP=$1
+
+	echo 1 > $CONFIGFS_DIR/$CHIP/live || fail "Unable to enable the chip"
+}
+
+disable_chip() {
+	local CHIP=$1
+
+	echo 0 > $CONFIGFS_DIR/$CHIP/live || fail "Unable to disable the chip"
+}
+
+configfs_chip_name() {
+	local CHIP="$1"
+
+	cat $CONFIGFS_DIR/$CHIP/chip_name 2> /dev/null || return 1
+}
+
+configfs_dev_name() {
+	local CHIP="$1"
+
+	cat $CONFIGFS_DIR/$CHIP/dev_name 2> /dev/null || return 1
+}
+
+get_chip_num_lines() {
+	local CHIP="$1"
+
+	$BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` num-lines
+}
+
+get_chip_label() {
+	local CHIP="$1"
+
+	$BASE_DIR/gpio-chip-info /dev/`configfs_chip_name $CHIP` label
+}
+
+get_line_name() {
+	local CHIP="$1"
+	local OFFSET="$2"
+
+	$BASE_DIR/gpio-line-name /dev/`configfs_chip_name $CHIP` $OFFSET
+}
+
+sysfs_set_pull() {
+	local CHIP="$1"
+	local OFFSET="$2"
+	local PULL="$3"
+	local SYSFSPATH="/sys/devices/platform/`configfs_dev_name $CHIP`/control/gpio$OFFSET"
+
+	echo $PULL > $SYSFSPATH || fail "Unable to set line pull in sysfs"
+}
+
+# Load the gpio-sim module. This will pull in configfs if needed too.
+modprobe gpio-sim || skip "unable to load the gpio-sim module"
+# Make sure configfs is mounted at /sys/kernel/config. Wait a bit if needed.
+for IDX in `seq 5`; do
+	if [ "$IDX" -eq "5" ]; then
+		skip "configfs not mounted at /sys/kernel/config"
+	fi
+
+	mountpoint -q /sys/kernel/config && break
+	sleep 0.1
+done
+# If the module was already loaded: remove all previous chips
+configfs_cleanup
+
+trap "exit 1" SIGTERM SIGINT
+trap configfs_cleanup EXIT
+
+echo "1. chip_name and dev_name attributes"
+
+echo "1.1. Chip name is communicated to user"
+create_chip chip
+enable_chip chip
+test -n `cat $CONFIGFS_DIR/chip/chip_name` || fail "chip_name doesn't work"
+remove_chip chip
+
+echo "1.2. chip_name returns 'none' if the chip is still pending"
+create_chip chip
+test "`cat $CONFIGFS_DIR/chip/chip_name`" = "none" || fail "chip_name doesn't return 'none' for a pending chip"
+remove_chip chip
+
+echo "1.3. Device name is communicated to user"
+create_chip chip
+enable_chip chip
+test -n `cat $CONFIGFS_DIR/chip/dev_name` || fail "dev_name doesn't work"
+remove_chip chip
+
+echo "2. Creating and configuring simulated chips"
+
+echo "2.1. Default number of lines is 1"
+create_chip chip
+enable_chip chip
+test "`get_chip_num_lines chip`" = "1" || fail "default number of lines is not 1"
+remove_chip chip
+
+echo "2.2. Number of lines can be specified"
+create_chip chip
+set_num_lines chip 16
+enable_chip chip
+test "`get_chip_num_lines chip`" = "16" || fail "number of lines is not 16"
+remove_chip chip
+
+echo "2.3. Label can be set"
+create_chip chip
+set_label chip foobar
+enable_chip chip
+test "`get_chip_label chip`" = "foobar" || fail "label is incorrect"
+remove_chip chip
+
+echo "2.4. Label can be left empty"
+create_chip chip
+enable_chip chip
+test -z "`cat $CONFIGFS_DIR/chip/label`" || fail "label is not empty"
+remove_chip chip
+
+echo "2.5. Line names can be configured"
+create_chip chip
+set_num_lines chip 16
+set_line_name chip 0 foo
+set_line_name chip 2 bar
+enable_chip chip
+test "`get_line_name chip 0`" = "foo" || fail "line name is incorrect"
+test "`get_line_name chip 2`" = "bar" || fail "line name is incorrect"
+remove_chip chip
+
+echo "2.6. Line name can remain unused"
+create_chip chip
+set_num_lines chip 2
+set_line_name chip 5 foobar
+enable_chip chip
+test "`get_line_name chip 0`" = "" || fail "line name is incorrect"
+test "`get_line_name chip 1`" = "" || fail "line name is incorrect"
+remove_chip chip
+
+echo "2.7. Line configfs directory names are sanitized"
+create_chip chip
+mkdir $CONFIGFS_DIR/chip/line12foobar 2> /dev/null && fail "invalid configfs line name accepted"
+mkdir $CONFIGFS_DIR/chip/line_no_offset 2> /dev/null && fail "invalid configfs line name accepted"
+remove_chip chip
+
+echo "2.8. Multiple chips can be created"
+CHIPS="chip0 chip1 chip2"
+for CHIP in $CHIPS; do
+	create_chip $CHIP
+	enable_chip $CHIP
+done
+for CHIP in $CHIPS; do
+	remove_chip $CHIP
+done
+
+echo "3. Controlling simulated chips"
+
+echo "3.3. Pull can be set over sysfs"
+create_chip chip
+set_num_lines chip 8
+enable_chip chip
+sysfs_set_pull chip 0 1
+$BASE_DIR/gpio-mockup-cdev /dev/`configfs_chip_name chip` 0
+test "$?" = "1" || fail "pull set incorrectly"
+sysfs_set_pull chip 0 0
+$BASE_DIR/gpio-mockup-cdev /dev/`configfs_chip_name chip` 1
+test "$?" = "0" || fail "pull set incorrectly"
+remove_chip chip
+
+echo "3.4. Incorrect input in sysfs is rejected"
+create_chip chip
+set_num_lines chip 8
+enable_chip chip
+SYSFS_PATH="/sys/devices/platform/`configfs_dev_name chip`/control/gpio0"
+echo 2 > $SYSFS_PATH 2> /dev/null && fail "invalid input not detected"
+remove_chip chip
+
+echo "4. Simulated GPIO chips are functional"
+
+echo "4.1. Values can be read from sysfs"
+create_chip chip
+set_num_lines chip 8
+enable_chip chip
+SYSFS_PATH="/sys/devices/platform/`configfs_dev_name chip`/control/gpio0"
+test `cat $SYSFS_PATH` = "0" || fail "incorrect value read from sysfs"
+$BASE_DIR/gpio-mockup-cdev -s 1 /dev/`configfs_chip_name chip` 0 &
+sleep 0.1 # FIXME Any better way?
+test `cat $SYSFS_PATH` = "1" || fail "incorrect value read from sysfs"
+kill $!
+remove_chip chip
+
+echo "4.2. Bias settings work correctly"
+create_chip chip
+set_num_lines chip 8
+enable_chip chip
+$BASE_DIR/gpio-mockup-cdev -b pull-up /dev/`configfs_chip_name chip` 0
+test `cat $SYSFS_PATH` = "1" || fail "bias setting does not work"
+remove_chip chip
+
+echo "GPIO $MODULE test PASS"
-- 
2.25.1

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Andy Shevchenko]

On Thu, Nov 18, 2021 at 03:51:38PM +0100, Bartosz Golaszewski wrote:
> This is another shot at the gpio-sim testing module. As there was no
> reasoning with configfs maintainers for many months, this time the whole
> concept of committable items has been dropped. Instead, each configfs
> chip item (or rather a group - more on that later) exposes a new
> attribute called 'live'. Writing 1 to it brings the chip on-line
> (registers the platform device) and writing 0 tears it down.
> 
> There are some caveats to that approach - for example: we can't block
> the user-space from deleting chip items when chips are live but is just
> handled by silently destroying the chip device in the background.
> 
> Andy (rightfully) pointed out that parsing of the lists of line names is
> awkward so in this iteration it's been replaced by a system that is more
> elegant and will allow to easily extend configuration options for
> specific GPIO lines. This is achieved by turning the chip's configfs
> item into a configfs group and allowing the user-space to create
> additional items inside it. The items must be called line<offset> (e.g.
> line0, line12 etc.) where the offset part indicates to the module the
> offset for which given item stores the configuration for. Within each
> such line item, there are additional attributes that allow specifying
> configuration for specific lines. Currently we only support the 'name'
> attribute but I plan to extend that to support GPIO hogging too.

One question here. Since you know how the driver looks like in both cases
(with and without committable items), would it be possible to modify what
you proposed here to the former one in case ConfigFS gains the feature?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Bartosz Golaszewski]

On Thu, Nov 18, 2021 at 4:50 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 18, 2021 at 03:51:38PM +0100, Bartosz Golaszewski wrote:
> > This is another shot at the gpio-sim testing module. As there was no
> > reasoning with configfs maintainers for many months, this time the whole
> > concept of committable items has been dropped. Instead, each configfs
> > chip item (or rather a group - more on that later) exposes a new
> > attribute called 'live'. Writing 1 to it brings the chip on-line
> > (registers the platform device) and writing 0 tears it down.
> >
> > There are some caveats to that approach - for example: we can't block
> > the user-space from deleting chip items when chips are live but is just
> > handled by silently destroying the chip device in the background.
> >
> > Andy (rightfully) pointed out that parsing of the lists of line names is
> > awkward so in this iteration it's been replaced by a system that is more
> > elegant and will allow to easily extend configuration options for
> > specific GPIO lines. This is achieved by turning the chip's configfs
> > item into a configfs group and allowing the user-space to create
> > additional items inside it. The items must be called line<offset> (e.g.
> > line0, line12 etc.) where the offset part indicates to the module the
> > offset for which given item stores the configuration for. Within each
> > such line item, there are additional attributes that allow specifying
> > configuration for specific lines. Currently we only support the 'name'
> > attribute but I plan to extend that to support GPIO hogging too.
>
> One question here. Since you know how the driver looks like in both cases
> (with and without committable items), would it be possible to modify what
> you proposed here to the former one in case ConfigFS gains the feature?
>

This would completely change the user interface unfortunately. We
could extend it but we would need to keep this one too most likely.

TBH I don't see the committable items merged anytime soon, and this is
GoodEnough®.

Bart

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Andy Shevchenko]

On Thu, Nov 18, 2021 at 05:37:02PM +0100, Bartosz Golaszewski wrote:
> On Thu, Nov 18, 2021 at 4:50 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, Nov 18, 2021 at 03:51:38PM +0100, Bartosz Golaszewski wrote:
> > > This is another shot at the gpio-sim testing module. As there was no
> > > reasoning with configfs maintainers for many months, this time the whole
> > > concept of committable items has been dropped. Instead, each configfs
> > > chip item (or rather a group - more on that later) exposes a new
> > > attribute called 'live'. Writing 1 to it brings the chip on-line
> > > (registers the platform device) and writing 0 tears it down.
> > >
> > > There are some caveats to that approach - for example: we can't block
> > > the user-space from deleting chip items when chips are live but is just
> > > handled by silently destroying the chip device in the background.
> > >
> > > Andy (rightfully) pointed out that parsing of the lists of line names is
> > > awkward so in this iteration it's been replaced by a system that is more
> > > elegant and will allow to easily extend configuration options for
> > > specific GPIO lines. This is achieved by turning the chip's configfs
> > > item into a configfs group and allowing the user-space to create
> > > additional items inside it. The items must be called line<offset> (e.g.
> > > line0, line12 etc.) where the offset part indicates to the module the
> > > offset for which given item stores the configuration for. Within each
> > > such line item, there are additional attributes that allow specifying
> > > configuration for specific lines. Currently we only support the 'name'
> > > attribute but I plan to extend that to support GPIO hogging too.
> >
> > One question here. Since you know how the driver looks like in both cases
> > (with and without committable items), would it be possible to modify what
> > you proposed here to the former one in case ConfigFS gains the feature?
> 
> This would completely change the user interface unfortunately. We
> could extend it but we would need to keep this one too most likely.
> 
> TBH I don't see the committable items merged anytime soon, and this is
> GoodEnough®.

Fine with me then!

Thanks for doing this all, I know it's a bit delayed in terms of getting
into upstream.

Btw, gpio-mockup testing scripts have an issue that the number of lines to
check overflow is hardcoded and since x86_64 switched to 1024 from 512 it
reveals the issue. Does gpio-sim solve this in a better way (like telling
to user space the ngpios, etc)?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Bartosz Golaszewski]

On Thu, Nov 18, 2021 at 5:59 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Nov 18, 2021 at 05:37:02PM +0100, Bartosz Golaszewski wrote:
> > On Thu, Nov 18, 2021 at 4:50 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > On Thu, Nov 18, 2021 at 03:51:38PM +0100, Bartosz Golaszewski wrote:
> > > > This is another shot at the gpio-sim testing module. As there was no
> > > > reasoning with configfs maintainers for many months, this time the whole
> > > > concept of committable items has been dropped. Instead, each configfs
> > > > chip item (or rather a group - more on that later) exposes a new
> > > > attribute called 'live'. Writing 1 to it brings the chip on-line
> > > > (registers the platform device) and writing 0 tears it down.
> > > >
> > > > There are some caveats to that approach - for example: we can't block
> > > > the user-space from deleting chip items when chips are live but is just
> > > > handled by silently destroying the chip device in the background.
> > > >
> > > > Andy (rightfully) pointed out that parsing of the lists of line names is
> > > > awkward so in this iteration it's been replaced by a system that is more
> > > > elegant and will allow to easily extend configuration options for
> > > > specific GPIO lines. This is achieved by turning the chip's configfs
> > > > item into a configfs group and allowing the user-space to create
> > > > additional items inside it. The items must be called line<offset> (e.g.
> > > > line0, line12 etc.) where the offset part indicates to the module the
> > > > offset for which given item stores the configuration for. Within each
> > > > such line item, there are additional attributes that allow specifying
> > > > configuration for specific lines. Currently we only support the 'name'
> > > > attribute but I plan to extend that to support GPIO hogging too.
> > >
> > > One question here. Since you know how the driver looks like in both cases
> > > (with and without committable items), would it be possible to modify what
> > > you proposed here to the former one in case ConfigFS gains the feature?
> >
> > This would completely change the user interface unfortunately. We
> > could extend it but we would need to keep this one too most likely.
> >
> > TBH I don't see the committable items merged anytime soon, and this is
> > GoodEnough®.
>
> Fine with me then!
>
> Thanks for doing this all, I know it's a bit delayed in terms of getting
> into upstream.
>
> Btw, gpio-mockup testing scripts have an issue that the number of lines to
> check overflow is hardcoded and since x86_64 switched to 1024 from 512 it
> reveals the issue. Does gpio-sim solve this in a better way (like telling
> to user space the ngpios, etc)?
>

Yeah the selftests need fixing now.

No, there's no fix for that in gpio-sim - probe() will just fail.
Which makes me think - maybe we should synchronously wait when writing
to 'live' for the probe to return (for instance setup a notifier) so
that we know if the chip probed correctly. Then we can notify the
user-space about the error destroy the device too.

Bart

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Kent Gibson]

On Fri, Nov 19, 2021 at 11:34:59AM +0100, Bartosz Golaszewski wrote:
> On Thu, Nov 18, 2021 at 5:59 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Thu, Nov 18, 2021 at 05:37:02PM +0100, Bartosz Golaszewski wrote:
> > > On Thu, Nov 18, 2021 at 4:50 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > >
> > > > On Thu, Nov 18, 2021 at 03:51:38PM +0100, Bartosz Golaszewski wrote:
> > > > > This is another shot at the gpio-sim testing module. As there was no
> > > > > reasoning with configfs maintainers for many months, this time the whole
> > > > > concept of committable items has been dropped. Instead, each configfs
> > > > > chip item (or rather a group - more on that later) exposes a new
> > > > > attribute called 'live'. Writing 1 to it brings the chip on-line
> > > > > (registers the platform device) and writing 0 tears it down.
> > > > >
> > > > > There are some caveats to that approach - for example: we can't block
> > > > > the user-space from deleting chip items when chips are live but is just
> > > > > handled by silently destroying the chip device in the background.
> > > > >
> > > > > Andy (rightfully) pointed out that parsing of the lists of line names is
> > > > > awkward so in this iteration it's been replaced by a system that is more
> > > > > elegant and will allow to easily extend configuration options for
> > > > > specific GPIO lines. This is achieved by turning the chip's configfs
> > > > > item into a configfs group and allowing the user-space to create
> > > > > additional items inside it. The items must be called line<offset> (e.g.
> > > > > line0, line12 etc.) where the offset part indicates to the module the
> > > > > offset for which given item stores the configuration for. Within each
> > > > > such line item, there are additional attributes that allow specifying
> > > > > configuration for specific lines. Currently we only support the 'name'
> > > > > attribute but I plan to extend that to support GPIO hogging too.
> > > >
> > > > One question here. Since you know how the driver looks like in both cases
> > > > (with and without committable items), would it be possible to modify what
> > > > you proposed here to the former one in case ConfigFS gains the feature?
> > >
> > > This would completely change the user interface unfortunately. We
> > > could extend it but we would need to keep this one too most likely.
> > >
> > > TBH I don't see the committable items merged anytime soon, and this is
> > > GoodEnough®.
> >
> > Fine with me then!
> >
> > Thanks for doing this all, I know it's a bit delayed in terms of getting
> > into upstream.
> >
> > Btw, gpio-mockup testing scripts have an issue that the number of lines to
> > check overflow is hardcoded and since x86_64 switched to 1024 from 512 it
> > reveals the issue. Does gpio-sim solve this in a better way (like telling
> > to user space the ngpios, etc)?
> >
> 
> Yeah the selftests need fixing now.
> 

No, there need to be new selftests added for your gpio-sim.
The existing selftests cover the gpio-mockup itself.

> No, there's no fix for that in gpio-sim - probe() will just fail.
> Which makes me think - maybe we should synchronously wait when writing
> to 'live' for the probe to return (for instance setup a notifier) so
> that we know if the chip probed correctly. Then we can notify the
> user-space about the error destroy the device too.
> 

+1 - need definite feedback to userspace that the change to the test setup
is in place so tests can proceed.  No arbitrary waits or waiting for
events from other subsystems like udev as we have to do with gpio-mockup.

Cheers,
Kent.

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Linus Walleij]

On Thu, Nov 18, 2021 at 3:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> v8 -> v9:
> - dropped the patches implementing committable-items and reworked the
>   driver to not use them
> - reworked the gpio-line-names property and configuring specific lines
>   in general
> - many smaller tweaks here and there

The series:
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Let's go with this.

Yours,
Linus Walleij

Re: [PATCH v9 0/4] gpio-sim: configfs-based GPIO simulator

[Bartosz Golaszewski]

On Mon, Nov 22, 2021 at 1:02 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Nov 18, 2021 at 3:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > v8 -> v9:
> > - dropped the patches implementing committable-items and reworked the
> >   driver to not use them
> > - reworked the gpio-line-names property and configuring specific lines
> >   in general
> > - many smaller tweaks here and there
>
> The series:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Let's go with this.
>
> Yours,
> Linus Walleij

Thanks but Kent and Andy are right, I'm about to send another version
that synchronously waits during `echo 1 > live` for the device to come
on-line.

Bart