Re: [PATCH v3 5/7] gpio: Add GPIO Aggregator/Repeater driver

[Ulrich Hecht <uli@fpond.eu>]

Thank you for this series!

> On November 27, 2019 at 9:42 AM Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> 
> 
> GPIO controllers are exported to userspace using /dev/gpiochip*
> character devices.  Access control to these devices is provided by
> standard UNIX file system permissions, on an all-or-nothing basis:
> either a GPIO controller is accessible for a user, or it is not.
> Currently no mechanism exists to control access to individual GPIOs.
> 
> Hence add a GPIO driver to aggregate existing GPIOs, and expose them as
> a new gpiochip.
> 
> This supports the following use cases:
>   1. Aggregating GPIOs using Sysfs
>      This is useful for implementing access control, and assigning a set
>      of GPIOs to a specific user or virtual machine.
> 
>   2. GPIO Repeater in Device Tree
>      This supports modelling e.g. GPIO inverters in DT.
> 
>   3. Generic GPIO Driver
>      This provides userspace access to a simple GPIO-operated device
>      described in DT, cfr. e.g. spidev for SPI-operated devices.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v3:
>   - Absorb GPIO forwarder,
>   - Integrate GPIO Repeater and Generic GPIO driver functionality,
>   - Use the aggregator parameters to create a GPIO lookup table instead
>     of an array of GPIO descriptors, which allows to simplify the code:
>       1. This removes the need for calling gpio_name_to_desc(),
>          gpiochip_find(), gpiochip_get_desc(), and gpiod_request(),
>       2. This allows the platform device to always use
>          devm_gpiod_get_index(), regardless of the origin of the GPIOs,
>   - Move parameter parsing from platform device probe to sysfs attribute
>     store, removing the need for platform data passing,
>   - Use more devm_*() functions to simplify cleanup,
>   - Add pr_fmt(),
>   - General refactoring.
> 
> v2:
>   - Add missing initialization of i in gpio_virt_agg_probe(),
>   - Update for removed .need_valid_mask field and changed
>     .init_valid_mask() signature,
>   - Drop "virtual", rename to gpio-aggregator,
>   - Drop bogus FIXME related to gpiod_set_transitory() expectations,
>   - Use new GPIO Forwarder Helper,
>   - Lift limit on the maximum number of GPIOs,
>   - Improve parsing:
>       - add support for specifying GPIOs by line name,
>       - add support for specifying GPIO chips by ID,
>       - add support for GPIO offset ranges,
>       - names and offset specifiers must be separated by whitespace,
>       - GPIO offsets must separated by spaces,
>   - Use str_has_prefix() and kstrtouint().
> ---
>  drivers/gpio/Kconfig           |  13 +
>  drivers/gpio/Makefile          |   1 +
>  drivers/gpio/gpio-aggregator.c | 587 +++++++++++++++++++++++++++++++++
>  3 files changed, 601 insertions(+)
>  create mode 100644 drivers/gpio/gpio-aggregator.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 8adffd42f8cb0559..36b6b57a6b05e906 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1507,6 +1507,19 @@ config GPIO_VIPERBOARD
>  
>  endmenu
>  
> +config GPIO_AGGREGATOR
> +	tristate "GPIO Aggregator/Repeater"
> +	help
> +	  Say yes here to enable the GPIO Aggregator and repeater, which

Inconsistent capitalization (Aggregator vs. repeater).

> +	  provides a way to aggregate and/or repeat existing GPIOs into a new
> +	  GPIO device.
> +	  This can serve the following purposes:
> +	    1. Assign a collection of GPIOs to a user, or export them to a
> +	       virtual machine,
> +	    2. Support GPIOs that are connected to a physical inverter,
> +	    3. Provide a generic driver for a GPIO-operated device, to be
> +               controlled from userspace using the GPIO chardev interface.
> +
>  config GPIO_MOCKUP
>  	tristate "GPIO Testing Driver"
>  	select IRQ_SIM
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 34eb8b2b12dd656c..f9971eeb1f32335f 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_GPIO_74XX_MMIO)		+= gpio-74xx-mmio.o
>  obj-$(CONFIG_GPIO_ADNP)			+= gpio-adnp.o
>  obj-$(CONFIG_GPIO_ADP5520)		+= gpio-adp5520.o
>  obj-$(CONFIG_GPIO_ADP5588)		+= gpio-adp5588.o
> +obj-$(CONFIG_GPIO_AGGREGATOR)		+= gpio-aggregator.o
>  obj-$(CONFIG_GPIO_ALTERA_A10SR)		+= gpio-altera-a10sr.o
>  obj-$(CONFIG_GPIO_ALTERA)  		+= gpio-altera.o
>  obj-$(CONFIG_GPIO_AMD8111)		+= gpio-amd8111.o
> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> new file mode 100644
> index 0000000000000000..873578c6f9683db8
> --- /dev/null
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -0,0 +1,587 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// GPIO Aggregator and Repeater
> +//
> +// Copyright (C) 2019 Glider bvba
> +
> +#define DRV_NAME       "gpio-aggregator"
> +#define pr_fmt(fmt)	DRV_NAME ": " fmt
> +
> +#include <linux/bitmap.h>
> +#include <linux/bitops.h>
> +#include <linux/ctype.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/machine.h>
> +#include <linux/idr.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/overflow.h>
> +#include <linux/platform_device.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> +
> +#include "gpiolib.h"
> +
> +
> +	/*
> +	 * GPIO Aggregator sysfs interface
> +	 */

Should this comment really be indented?

> +
> +struct gpio_aggregator {
> +	struct gpiod_lookup_table *lookups;
> +	struct platform_device *pdev;
> +	char args[];
> +};
> +
> +static DEFINE_MUTEX(gpio_aggregator_lock);	/* protects idr */
> +static DEFINE_IDR(gpio_aggregator_idr);
> +
> +static char *get_arg(char **args)
> +{
> +	char *start = *args, *end;
> +
> +	while (isspace(*start))
> +		start++;

"start = skip_spaces(start);", perhaps. (Looking at a grep through drivers/, I would say usage of while(isspace(...))... vs. skip_spaces() is about 60/40.)

> +
> +	if (!*start)
> +		return NULL;
> +
> +	if (*start == '"') {
> +		/* Quoted arg */
> +		end = strchr(++start, '"');
> +		if (!end)
> +			return ERR_PTR(-EINVAL);
> +	} else {
> +		/* Unquoted arg */
> +		for (end = start; *end && !isspace(*end); end++) ;
> +	}
> +
> +	if (*end)
> +		*end++ = '\0';
> +
> +	*args = end;
> +	return start;
> +}
> +
> +static bool isrange(const char *s)
> +{
> +	size_t n = strlen(s);
> +
> +	if (IS_ERR_OR_NULL(s))
> +		return false;
> +
> +	while (1) {
> +		n = strspn(s, "0123456789");
> +		if (!n)
> +			return false;
> +
> +		s += n;
> +
> +		switch (*s++) {
> +		case '\0':
> +			return true;
> +
> +		case '-':
> +		case ',':
> +			break;
> +
> +		default:
> +			return false;
> +		}
> +	}
> +}
> +
> +static int aggr_add_gpio(struct gpio_aggregator *aggr, const char *label,
> +			 int hwnum, unsigned int *n)
> +{
> +	struct gpiod_lookup_table *lookups;
> +
> +	lookups = krealloc(aggr->lookups, struct_size(lookups, table, *n + 2),
> +			   GFP_KERNEL);
> +	if (!lookups)
> +		return -ENOMEM;
> +
> +	lookups->table[*n].chip_label = label;
> +	lookups->table[*n].chip_hwnum = hwnum;
> +	lookups->table[*n].idx = *n;
> +
> +	(*n)++;
> +	memset(&lookups->table[*n], 0, sizeof(lookups->table[*n]));
> +
> +	aggr->lookups = lookups;
> +	return 0;
> +}
> +
> +static int aggr_parse(struct gpio_aggregator *aggr)
> +{
> +	char *name, *offsets, *first, *last, *next;
> +	unsigned int a, b, i, n = 0;

Too many single-letter variables. "first_index" and "last_index" instead of "a" and "b" would make it easier to understand.

> +	char *args = aggr->args;
> +	int error;
> +
> +	for (name = get_arg(&args), offsets = get_arg(&args); name;
> +	     offsets = get_arg(&args)) {
> +		if (IS_ERR(name)) {
> +			pr_err("Cannot get GPIO specifier: %ld\n",
> +			       PTR_ERR(name));
> +			return PTR_ERR(name);
> +		}
> +
> +		if (!isrange(offsets)) {
> +			/* Named GPIO line */
> +			error = aggr_add_gpio(aggr, name, -1, &n);

The "-1" magic could use a #define, IMO.

> +			if (error)
> +				return error;
> +
> +			name = offsets;
> +			continue;
> +		}
> +
> +		/* GPIO chip + offset(s) */
> +		for (first = offsets; *first; first = next) {
> +			next = strchrnul(first, ',');
> +			if (*next)
> +				*next++ = '\0';
> +
> +			last = strchr(first, '-');
> +			if (last)
> +				*last++ = '\0';
> +
> +			if (kstrtouint(first, 10, &a)) {
> +				pr_err("Cannot parse GPIO index %s\n", first);
> +				return -EINVAL;
> +			}
> +
> +			if (!last) {
> +				b = a;
> +			} else if (kstrtouint(last, 10, &b)) {
> +				pr_err("Cannot parse GPIO index %s\n", last);
> +				return -EINVAL;
> +			}
> +
> +			for (i = a; i <= b; i++) {
> +				error = aggr_add_gpio(aggr, name, i, &n);
> +				if (error)
> +					return error;
> +			}
> +		}
> +
> +		name = get_arg(&args);
> +	}
> +
> +	if (!n) {
> +		pr_err("No GPIOs specified\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t new_device_store(struct device_driver *driver, const char *buf,
> +				size_t count)
> +{
> +	struct gpio_aggregator *aggr;
> +	struct platform_device *pdev;
> +	int res, id;
> +
> +	/* kernfs guarantees string termination, so count + 1 is safe */
> +	aggr = kzalloc(sizeof(*aggr) + count + 1, GFP_KERNEL);
> +	if (!aggr)
> +		return -ENOMEM;
> +
> +	memcpy(aggr->args, buf, count + 1);
> +
> +	aggr->lookups = kzalloc(struct_size(aggr->lookups, table, 1),
> +				GFP_KERNEL);
> +	if (!aggr->lookups) {
> +		res = -ENOMEM;
> +		goto free_ga;
> +	}
> +
> +	mutex_lock(&gpio_aggregator_lock);
> +	id = idr_alloc(&gpio_aggregator_idr, aggr, 0, 0, GFP_KERNEL);
> +	mutex_unlock(&gpio_aggregator_lock);
> +
> +	if (id < 0) {
> +		res = id;
> +		goto free_table;
> +	}
> +
> +	aggr->lookups->dev_id = kasprintf(GFP_KERNEL, "%s.%d", DRV_NAME, id);
> +	if (!aggr->lookups) {
> +		res = -ENOMEM;
> +		goto remove_idr;
> +	}
> +
> +	res = aggr_parse(aggr);
> +	if (res)
> +		goto free_dev_id;
> +
> +	gpiod_add_lookup_table(aggr->lookups);
> +
> +	pdev = platform_device_register_simple(DRV_NAME, id, NULL, 0);
> +	if (IS_ERR(pdev)) {
> +		res = PTR_ERR(pdev);
> +		goto remove_table;
> +	}
> +
> +	aggr->pdev = pdev;
> +	return count;
> +
> +remove_table:
> +	gpiod_remove_lookup_table(aggr->lookups);
> +free_dev_id:
> +	kfree(aggr->lookups->dev_id);
> +remove_idr:
> +	mutex_lock(&gpio_aggregator_lock);
> +	idr_remove(&gpio_aggregator_idr, id);
> +	mutex_unlock(&gpio_aggregator_lock);
> +free_table:
> +	kfree(aggr->lookups);
> +free_ga:
> +	kfree(aggr);
> +	return res;
> +}
> +
> +static DRIVER_ATTR_WO(new_device);
> +
> +static void gpio_aggregator_free(struct gpio_aggregator *aggr)
> +{
> +	platform_device_unregister(aggr->pdev);
> +	gpiod_remove_lookup_table(aggr->lookups);
> +	kfree(aggr->lookups->dev_id);
> +	kfree(aggr->lookups);
> +	kfree(aggr);
> +}
> +
> +static ssize_t delete_device_store(struct device_driver *driver,
> +				   const char *buf, size_t count)
> +{
> +	struct gpio_aggregator *aggr;
> +	unsigned int id;
> +	int error;
> +
> +	if (!str_has_prefix(buf, DRV_NAME "."))
> +		return -EINVAL;
> +
> +	error = kstrtouint(buf + strlen(DRV_NAME "."), 10, &id);
> +	if (error)
> +		return error;
> +
> +	mutex_lock(&gpio_aggregator_lock);
> +	aggr = idr_remove(&gpio_aggregator_idr, id);
> +	mutex_unlock(&gpio_aggregator_lock);
> +	if (!aggr)
> +		return -ENOENT;
> +
> +	gpio_aggregator_free(aggr);
> +	return count;
> +}
> +static DRIVER_ATTR_WO(delete_device);
> +
> +static struct attribute *gpio_aggregator_attrs[] = {
> +	&driver_attr_new_device.attr,
> +	&driver_attr_delete_device.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(gpio_aggregator);
> +
> +static int __exit gpio_aggregator_idr_remove(int id, void *p, void *data)
> +{
> +	gpio_aggregator_free(p);
> +	return 0;
> +}
> +
> +static void __exit gpio_aggregator_remove_all(void)
> +{
> +	mutex_lock(&gpio_aggregator_lock);
> +	idr_for_each(&gpio_aggregator_idr, gpio_aggregator_idr_remove, NULL);
> +	idr_destroy(&gpio_aggregator_idr);
> +	mutex_unlock(&gpio_aggregator_lock);
> +}
> +
> +
> +	/*
> +	 *  Common GPIO Forwarder
> +	 */
> +
> +struct gpiochip_fwd {
> +	struct gpio_chip chip;
> +	struct gpio_desc **descs;
> +	union {
> +		struct mutex mlock;	/* protects tmp[] if can_sleep */
> +		spinlock_t slock;	/* protects tmp[] if !can_sleep */
> +	};
> +	unsigned long tmp[];		/* values and descs for multiple ops */
> +};
> +
> +static int gpio_fwd_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	return gpiod_get_direction(fwd->descs[offset]);
> +}
> +
> +static int gpio_fwd_direction_input(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	return gpiod_direction_input(fwd->descs[offset]);
> +}
> +
> +static int gpio_fwd_direction_output(struct gpio_chip *chip,
> +				     unsigned int offset, int value)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	return gpiod_direction_output(fwd->descs[offset], value);
> +}
> +
> +static int gpio_fwd_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	return gpiod_get_value(fwd->descs[offset]);
> +}
> +
> +static int gpio_fwd_get_multiple(struct gpio_chip *chip, unsigned long *mask,
> +				 unsigned long *bits)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +	unsigned long *values, flags;
> +	struct gpio_desc **descs;
> +	unsigned int i, j = 0;
> +	int error;
> +
> +	if (chip->can_sleep)
> +		mutex_lock(&fwd->mlock);
> +	else
> +		spin_lock_irqsave(&fwd->slock, flags);
> +
> +	values = &fwd->tmp[0];
> +	bitmap_clear(values, 0, fwd->chip.ngpio);
> +	descs = (void *)&fwd->tmp[BITS_TO_LONGS(fwd->chip.ngpio)];

This use of the tmp array, which I presume serves to avoid extra memory allocations at the time of device creation, is odd enough to deserve a comment.

> +
> +	for_each_set_bit(i, mask, fwd->chip.ngpio)
> +		descs[j++] = fwd->descs[i];
> +
> +	error = gpiod_get_array_value(j, descs, NULL, values);
> +	if (!error) {
> +		j = 0;
> +		for_each_set_bit(i, mask, fwd->chip.ngpio)
> +			__assign_bit(i, bits, test_bit(j++, values));
> +	}
> +
> +	if (chip->can_sleep)
> +		mutex_unlock(&fwd->mlock);
> +	else
> +		spin_unlock_irqrestore(&fwd->slock, flags);
> +
> +	return error;
> +}
> +
> +static void gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	gpiod_set_value(fwd->descs[offset], value);
> +}
> +
> +static void gpio_fwd_set_multiple(struct gpio_chip *chip, unsigned long *mask,
> +				  unsigned long *bits)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +	unsigned long *values, flags;
> +	struct gpio_desc **descs;
> +	unsigned int i, j = 0;
> +
> +	if (chip->can_sleep)
> +		mutex_lock(&fwd->mlock);
> +	else
> +		spin_lock_irqsave(&fwd->slock, flags);
> +
> +	values = &fwd->tmp[0];
> +	descs = (void *)&fwd->tmp[BITS_TO_LONGS(fwd->chip.ngpio)];
> +
> +	for_each_set_bit(i, mask, fwd->chip.ngpio) {
> +		__assign_bit(j, values, test_bit(i, bits));
> +		descs[j++] = fwd->descs[i];
> +	}
> +
> +	gpiod_set_array_value(j, descs, NULL, values);
> +
> +	if (chip->can_sleep)
> +		mutex_unlock(&fwd->mlock);
> +	else
> +		spin_unlock_irqrestore(&fwd->slock, flags);
> +}
> +
> +static int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset,
> +			       unsigned long config)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +
> +	chip = fwd->descs[offset]->gdev->chip;
> +	if (chip->set_config)
> +		return chip->set_config(chip, offset, config);
> +
> +	return -ENOTSUPP;
> +}
> +
> +static int gpio_fwd_init_valid_mask(struct gpio_chip *chip,
> +				    unsigned long *valid_mask,
> +				    unsigned int ngpios)
> +{
> +	struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
> +	unsigned int i;
> +
> +	for (i = 0; i < ngpios; i++) {
> +		if (!gpiochip_line_is_valid(fwd->descs[i]->gdev->chip,
> +					    gpio_chip_hwgpio(fwd->descs[i])))
> +			clear_bit(i, valid_mask);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * gpiochip_fwd_create() - Create a new GPIO forwarder
> + * @dev: Parent device pointer
> + * @ngpios: Number of GPIOs in the forwarder.
> + * @descs: Array containing the GPIO descriptors to forward to.
> + *         This array must contain @ngpios entries, and must not be deallocated
> + *         before the forwarder has been destroyed again.
> + *
> + * This function creates a new gpiochip, which forwards all GPIO operations to
> + * the passed GPIO descriptors.
> + *
> + * Return: An opaque object pointer, or an ERR_PTR()-encoded negative error
> + *         code on failure.
> + */
> +static struct gpiochip_fwd *gpiochip_fwd_create(struct device *dev,
> +						unsigned int ngpios,
> +						struct gpio_desc *descs[])
> +{
> +	const char *label = dev_name(dev);
> +	struct gpiochip_fwd *fwd;
> +	struct gpio_chip *chip;
> +	unsigned int i;
> +	int error;
> +
> +	fwd = devm_kzalloc(dev, struct_size(fwd, tmp,
> +			   BITS_TO_LONGS(ngpios) + ngpios), GFP_KERNEL);
> +	if (!fwd)
> +		return ERR_PTR(-ENOMEM);
> +
> +	chip = &fwd->chip;
> +
> +	for (i = 0; i < ngpios; i++) {
> +		dev_dbg(dev, "gpio %u => gpio-%d (%s)\n", i,
> +			desc_to_gpio(descs[i]), descs[i]->label ? : "?");
> +
> +		if (gpiod_cansleep(descs[i]))
> +			chip->can_sleep = true;
> +		if (descs[i]->gdev->chip->set_config)
> +			chip->set_config = gpio_fwd_set_config;
> +		if (descs[i]->gdev->chip->init_valid_mask)
> +			chip->init_valid_mask = gpio_fwd_init_valid_mask;
> +	}
> +
> +	chip->label = label;
> +	chip->parent = dev;
> +	chip->owner = THIS_MODULE;
> +	chip->get_direction = gpio_fwd_get_direction;
> +	chip->direction_input = gpio_fwd_direction_input;
> +	chip->direction_output = gpio_fwd_direction_output;
> +	chip->get = gpio_fwd_get;
> +	chip->get_multiple = gpio_fwd_get_multiple;
> +	chip->set = gpio_fwd_set;
> +	chip->set_multiple = gpio_fwd_set_multiple;
> +	chip->base = -1;
> +	chip->ngpio = ngpios;
> +	fwd->descs = descs;
> +
> +	if (chip->can_sleep)
> +		mutex_init(&fwd->mlock);
> +	else
> +		spin_lock_init(&fwd->slock);
> +
> +	error = devm_gpiochip_add_data(dev, chip, fwd);
> +	if (error)
> +		return ERR_PTR(error);
> +
> +	return fwd;
> +}
> +
> +
> +	/*
> +	 *  Common GPIO Aggregator/Repeater platform device
> +	 */
> +
> +static int gpio_aggregator_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct gpio_desc **descs;
> +	struct gpiochip_fwd *fwd;
> +	int i, n;
> +
> +	n = gpiod_count(dev, NULL);
> +	if (n < 0)
> +		return n;
> +
> +	descs = devm_kmalloc_array(dev, n, sizeof(*descs), GFP_KERNEL);
> +	if (!descs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < n; i++) {
> +		descs[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
> +		if (IS_ERR(descs[i]))
> +			return PTR_ERR(descs[i]);
> +	}
> +
> +	fwd = gpiochip_fwd_create(dev, n, descs);
> +	if (IS_ERR(fwd))
> +		return PTR_ERR(fwd);
> +
> +	platform_set_drvdata(pdev, fwd);
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id gpio_aggregator_dt_ids[] = {
> +	{ .compatible = "gpio-repeater" },
> +	/*
> +	 * Add GPIO-operated devices controlled from userspace below,
> +	 * or use "driver_override" in sysfs
> +	 */
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, gpio_aggregator_dt_ids);
> +#endif
> +
> +static struct platform_driver gpio_aggregator_driver = {
> +	.probe = gpio_aggregator_probe,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.groups = gpio_aggregator_groups,
> +		.of_match_table = of_match_ptr(gpio_aggregator_dt_ids),
> +	},
> +};
> +
> +static int __init gpio_aggregator_init(void)
> +{
> +	return platform_driver_register(&gpio_aggregator_driver);
> +}
> +module_init(gpio_aggregator_init);
> +
> +static void __exit gpio_aggregator_exit(void)
> +{
> +	gpio_aggregator_remove_all();
> +	platform_driver_unregister(&gpio_aggregator_driver);
> +}
> +module_exit(gpio_aggregator_exit);
> +
> +MODULE_AUTHOR("Geert Uytterhoeven <geert+renesas@glider.be>");
> +MODULE_DESCRIPTION("GPIO Aggregator/Repeater");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.17.1
>

CU
Uli