[PATCH 2/2] gpio: adg1712: add driver support

Antoniu Miclaus posted 2 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH 2/2] gpio: adg1712: add driver support
Posted by Antoniu Miclaus 3 months, 1 week ago
Add driver support for the ADG1712, which contains four independent
single-pole/single-throw (SPST) switches and operates with a
low-voltage single supply range from +1.08V to +5.5V or a low-voltage
dual supply range from ±1.08V to ±2.75V.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 drivers/gpio/Kconfig        |   9 +++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-adg1712.c | 146 ++++++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 drivers/gpio/gpio-adg1712.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 7ee3afbc2b05..3fac05823eae 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -157,6 +157,15 @@ config GPIO_74XX_MMIO
 	    8 bits:	74244 (Input), 74273 (Output)
 	    16 bits:	741624 (Input), 7416374 (Output)
 
+config GPIO_ADG1712
+	tristate "Analog Devices ADG1712 quad SPST switch GPIO driver"
+	depends on GPIOLIB
+	help
+	  GPIO driver for Analog Devices ADG1712 quad single-pole,
+	  single-throw (SPST) switch. The driver provides a GPIO controller
+	  interface where each GPIO line controls one of the four independent
+	  analog switches on the ADG1712.
+
 config GPIO_ALTERA
 	tristate "Altera GPIO"
 	select GPIOLIB_IRQCHIP
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ec296fa14bfd..9043d2d07a15 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_GPIO_104_IDI_48)		+= gpio-104-idi-48.o
 obj-$(CONFIG_GPIO_104_IDIO_16)		+= gpio-104-idio-16.o
 obj-$(CONFIG_GPIO_74X164)		+= gpio-74x164.o
 obj-$(CONFIG_GPIO_74XX_MMIO)		+= gpio-74xx-mmio.o
+obj-$(CONFIG_GPIO_ADG1712)		+= gpio-adg1712.o
 obj-$(CONFIG_GPIO_ADNP)			+= gpio-adnp.o
 obj-$(CONFIG_GPIO_ADP5520)		+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5585)		+= gpio-adp5585.o
diff --git a/drivers/gpio/gpio-adg1712.c b/drivers/gpio/gpio-adg1712.c
new file mode 100644
index 000000000000..f8d3481ac9d0
--- /dev/null
+++ b/drivers/gpio/gpio-adg1712.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices ADG1712 quad SPST switch GPIO driver
+ *
+ * Copyright 2025 Analog Devices Inc.
+ *
+ * Author: Antoniu Miclaus <antoniu.miclaus@analog.com>
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+
+#define ADG1712_NUM_GPIOS	4
+
+struct adg1712 {
+	struct gpio_chip chip;
+	struct gpio_desc *switch_gpios[ADG1712_NUM_GPIOS];
+};
+
+static int adg1712_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int adg1712_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	return -EINVAL;
+}
+
+static int adg1712_direction_output(struct gpio_chip *chip, unsigned int offset,
+				    int value)
+{
+	struct adg1712 *adg1712 = gpiochip_get_data(chip);
+
+	if (offset >= ADG1712_NUM_GPIOS)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(adg1712->switch_gpios[offset], value);
+	return 0;
+}
+
+static int adg1712_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct adg1712 *adg1712 = gpiochip_get_data(chip);
+
+	if (offset >= ADG1712_NUM_GPIOS)
+		return -EINVAL;
+
+	gpiod_set_value_cansleep(adg1712->switch_gpios[offset], value);
+	return 0;
+}
+
+static int adg1712_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct adg1712 *adg1712 = gpiochip_get_data(chip);
+
+	if (offset >= ADG1712_NUM_GPIOS)
+		return -EINVAL;
+
+	return gpiod_get_value_cansleep(adg1712->switch_gpios[offset]);
+}
+
+static int adg1712_set_multiple(struct gpio_chip *chip, unsigned long *mask,
+				 unsigned long *bits)
+{
+	struct adg1712 *adg1712 = gpiochip_get_data(chip);
+	int i;
+
+	for_each_set_bit(i, mask, ADG1712_NUM_GPIOS) {
+		gpiod_set_value_cansleep(adg1712->switch_gpios[i],
+					 test_bit(i, bits));
+	}
+
+	return 0;
+}
+
+static const struct gpio_chip adg1712_gpio_chip = {
+	.label			= "adg1712",
+	.owner			= THIS_MODULE,
+	.get_direction		= adg1712_get_direction,
+	.direction_input	= adg1712_direction_input,
+	.direction_output	= adg1712_direction_output,
+	.get			= adg1712_get,
+	.set			= adg1712_set,
+	.set_multiple		= adg1712_set_multiple,
+	.base			= -1,
+	.ngpio			= ADG1712_NUM_GPIOS,
+	.can_sleep		= true,
+};
+
+static int adg1712_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct adg1712 *adg1712;
+	int ret, i;
+	char gpio_name[16];
+
+	adg1712 = devm_kzalloc(dev, sizeof(*adg1712), GFP_KERNEL);
+	if (!adg1712)
+		return -ENOMEM;
+
+	adg1712->chip = adg1712_gpio_chip;
+	adg1712->chip.parent = dev;
+
+	for (i = 0; i < ADG1712_NUM_GPIOS; i++) {
+		snprintf(gpio_name, sizeof(gpio_name), "switch%d", i + 1);
+		adg1712->switch_gpios[i] = devm_gpiod_get(dev, gpio_name,
+							  GPIOD_OUT_LOW);
+		if (IS_ERR(adg1712->switch_gpios[i]))
+			return dev_err_probe(dev, PTR_ERR(adg1712->switch_gpios[i]),
+					     "failed to get %s gpio\n", gpio_name);
+	}
+
+	ret = devm_gpiochip_add_data(dev, &adg1712->chip, adg1712);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to add gpio chip\n");
+
+	dev_info(dev, "ADG1712 %u-GPIO expander registered\n",
+		 adg1712->chip.ngpio);
+
+	return 0;
+}
+
+static const struct of_device_id adg1712_dt_ids[] = {
+	{ .compatible = "adi,adg1712", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adg1712_dt_ids);
+
+static struct platform_driver adg1712_driver = {
+	.driver = {
+		.name = "adg1712",
+		.of_match_table = adg1712_dt_ids,
+	},
+	.probe = adg1712_probe,
+};
+module_platform_driver(adg1712_driver);
+
+MODULE_DESCRIPTION("Analog Devices ADG1712 quad SPST switch GPIO driver");
+MODULE_AUTHOR("Antoniu Miclaus <antoniu.miclaus@analog.com>");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Linus Walleij 3 months ago
Hi Antoniu,

thanks for your patch!

On Fri, Oct 31, 2025 at 5:08 PM Antoniu Miclaus
<antoniu.miclaus@analog.com> wrote:

> Add driver support for the ADG1712, which contains four independent
> single-pole/single-throw (SPST) switches and operates with a
> low-voltage single supply range from +1.08V to +5.5V or a low-voltage
> dual supply range from ±1.08V to ±2.75V.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

So tying into the binding discussion:

GPIO means "general purpose input/output".

I am really confused as whether this is:

- General purpose - seems to be for the purpose of switching
  currents and nothing else.

- Input/Output - It's switching something else and not inputting
  or outputting anything and this makes the driver look strange.

Yours,
Linus Walleij
Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Nuno Sá 3 months ago
On Mon, 2025-11-10 at 11:30 +0100, Linus Walleij wrote:
> Hi Antoniu,
> 
> thanks for your patch!
> 
> On Fri, Oct 31, 2025 at 5:08 PM Antoniu Miclaus
> <antoniu.miclaus@analog.com> wrote:
> 
> > Add driver support for the ADG1712, which contains four independent
> > single-pole/single-throw (SPST) switches and operates with a
> > low-voltage single supply range from +1.08V to +5.5V or a low-voltage
> > dual supply range from ±1.08V to ±2.75V.
> > 
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> 
> So tying into the binding discussion:
> 
> GPIO means "general purpose input/output".
> 
> I am really confused as whether this is:
> 
> - General purpose - seems to be for the purpose of switching
>   currents and nothing else.
> 
> - Input/Output - It's switching something else and not inputting
>   or outputting anything and this makes the driver look strange.
> 
> 

Not the first time a part like this pops up [1]. At the time, the final
conclusion was to go with gpiolib. Naturally you can think otherwise now :)

Also, it looks like that series did not went anywhere (I'll probably ping the author internally)


[1]: https://lore.kernel.org/linux-gpio/20250213-for_upstream-v2-2-ec4eff3b3cd5@analog.com/

- Nuno Sá
Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Linus Walleij 3 months ago
On Mon, Nov 10, 2025 at 1:32 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> On Mon, 2025-11-10 at 11:30 +0100, Linus Walleij wrote:
> > Hi Antoniu,
> >
> > thanks for your patch!
> >
> > On Fri, Oct 31, 2025 at 5:08 PM Antoniu Miclaus
> > <antoniu.miclaus@analog.com> wrote:
> >
> > > Add driver support for the ADG1712, which contains four independent
> > > single-pole/single-throw (SPST) switches and operates with a
> > > low-voltage single supply range from +1.08V to +5.5V or a low-voltage
> > > dual supply range from ±1.08V to ±2.75V.
> > >
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> >
> > So tying into the binding discussion:
> >
> > GPIO means "general purpose input/output".
> >
> > I am really confused as whether this is:
> >
> > - General purpose - seems to be for the purpose of switching
> >   currents and nothing else.
> >
> > - Input/Output - It's switching something else and not inputting
> >   or outputting anything and this makes the driver look strange.
> >
> >
>
> Not the first time a part like this pops up [1]. At the time, the final
> conclusion was to go with gpiolib. Naturally you can think otherwise now :)

I think we might wanna go with gpiolib for the Linux internals, maybe
we want to add some kind of awareness or flag in gpiolib that this is
a switch and not an output we can control the level of?

I could think of this:

- Make .get() and .set() in struct gpio_chip return -ENOTIMP
  no getting and setting these "lines" because we really cannot
  control that, these lines will have the level of whatever is on
  the line we are switching.

- Implement .set_config() and implement the generic pin
  control property PIN_CONFIG_OUTPUT_ENABLE as 1
  to switch "on" and 0 for switch "off".
  See include/linux/pinctrl/pinconf-generic.h

This makes it possible to use the gpiolib in a way that is
non-ambiguous.

We might want to add consumer helpers in
include/linux/gpio/consumer.h such as:

#include <linux/pinctrl/pinconf-generic.h>

int gpiod_switch_enable(struct gpio_desc *desc)
{
   unsigned long cfg = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 1);

   return gpiod_set_config(desc, cfg);
}

int gpiod_switch_disable(struct gpio_desc *desc)
{
   unsigned long cfg = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0);

   return gpiod_set_config(desc, cfg);
}

See e.g. rtd_gpio_set_config() in drivers/gpio/gpio-rtd.c for
an example of how the GPIO driver can unpack and handle
generic .set_config() options like this.

The binding however needs to be something separate like a proper switch binding
I think or we will confuse other operating systems.

Yours,
Linus Walleij
Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Nuno Sá 3 months ago
On Tue, 2025-11-11 at 12:05 +0100, Linus Walleij wrote:
> On Mon, Nov 10, 2025 at 1:32 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> > On Mon, 2025-11-10 at 11:30 +0100, Linus Walleij wrote:
> > > Hi Antoniu,
> > > 
> > > thanks for your patch!
> > > 
> > > On Fri, Oct 31, 2025 at 5:08 PM Antoniu Miclaus
> > > <antoniu.miclaus@analog.com> wrote:
> > > 
> > > > Add driver support for the ADG1712, which contains four independent
> > > > single-pole/single-throw (SPST) switches and operates with a
> > > > low-voltage single supply range from +1.08V to +5.5V or a low-voltage
> > > > dual supply range from ±1.08V to ±2.75V.
> > > > 
> > > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > 
> > > So tying into the binding discussion:
> > > 
> > > GPIO means "general purpose input/output".
> > > 
> > > I am really confused as whether this is:
> > > 
> > > - General purpose - seems to be for the purpose of switching
> > >   currents and nothing else.
> > > 
> > > - Input/Output - It's switching something else and not inputting
> > >   or outputting anything and this makes the driver look strange.
> > > 
> > > 
> > 
> > Not the first time a part like this pops up [1]. At the time, the final
> > conclusion was to go with gpiolib. Naturally you can think otherwise now :)
> 
> I think we might wanna go with gpiolib for the Linux internals, maybe
> we want to add some kind of awareness or flag in gpiolib that this is
> a switch and not an output we can control the level of?
> 
> I could think of this:
> 
> - Make .get() and .set() in struct gpio_chip return -ENOTIMP
>   no getting and setting these "lines" because we really cannot
>   control that, these lines will have the level of whatever is on
>   the line we are switching.
> 
> - Implement .set_config() and implement the generic pin
>   control property PIN_CONFIG_OUTPUT_ENABLE as 1
>   to switch "on" and 0 for switch "off".
>   See include/linux/pinctrl/pinconf-generic.h
> 
> This makes it possible to use the gpiolib in a way that is
> non-ambiguous.
> 

The above makes sense to me. I'll let Antoniu take it from here and check if 
the above fits the usecases he is aware of. Not sure if it makes sense for a piece
of HW like this but if the usecase is for userspace to control the on/off states,
then I guess we would need .get() and .set(). Or some kind of "frontend" driver
making use of the consumer helpers.

Thanks!
- Nuno Sá

> We might want to add consumer helpers in
> include/linux/gpio/consumer.h such as:
> 
> #include <linux/pinctrl/pinconf-generic.h>
> 
> int gpiod_switch_enable(struct gpio_desc *desc)
> {
>    unsigned long cfg = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 1);
> 
>    return gpiod_set_config(desc, cfg);
> }
> 
> int gpiod_switch_disable(struct gpio_desc *desc)
> {
>    unsigned long cfg = pinconf_to_config_packed(PIN_CONFIG_OUTPUT_ENABLE, 0);
> 
>    return gpiod_set_config(desc, cfg);
> }
> 
> See e.g. rtd_gpio_set_config() in drivers/gpio/gpio-rtd.c for
> an example of how the GPIO driver can unpack and handle
> generic .set_config() options like this.
> 
> The binding however needs to be something separate like a proper switch binding
> I think or we will confuse other operating systems.
> 
> Yours,
> Linus Walleij
Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Linus Walleij 2 months, 3 weeks ago
On Tue, Nov 11, 2025 at 5:01 PM Nuno Sá <noname.nuno@gmail.com> wrote:

[Me]
>> - Implement .set_config() and implement the generic pin
>>   control property PIN_CONFIG_OUTPUT_ENABLE as 1
>>   to switch "on" and 0 for switch "off".
>>   See include/linux/pinctrl/pinconf-generic.h

> The above makes sense to me. I'll let Antoniu take it from here and check if
> the above fits the usecases he is aware of. Not sure if it makes sense for a piece
> of HW like this but if the usecase is for userspace to control the on/off states,
> then I guess we would need .get() and .set(). Or some kind of "frontend" driver
> making use of the consumer helpers.

There is already GPIO_V2_LINE_SET_CONFIG_IOCTL
in <uapi/linux/gpio.h> so setting configs from userspace is no issue,
just use the character device.

You will need to add I think two new config flags for userspace:
GPIO_V2_LINE_FLAG_OUTPUT_ENABLE
GPIO_V2_LINE_FLAG_OUTPUT_DISABLE

And update gpio_v2_line_config_flags_to_desc_flags() in
drivers/gpio/gpiolib-cdev.c accordingly.

Then you probably want some tests or examples in libgpiod to make
sure userspace is fine. Bartosz knows all about how to do this.

Yours,
Linus Walleij
Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Nuno Sá 2 months, 3 weeks ago
On Tue, 2025-11-18 at 23:54 +0100, Linus Walleij wrote:
> On Tue, Nov 11, 2025 at 5:01 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> 
> [Me]
> > > - Implement .set_config() and implement the generic pin
> > >   control property PIN_CONFIG_OUTPUT_ENABLE as 1
> > >   to switch "on" and 0 for switch "off".
> > >   See include/linux/pinctrl/pinconf-generic.h
> 
> > The above makes sense to me. I'll let Antoniu take it from here and check if
> > the above fits the usecases he is aware of. Not sure if it makes sense for a piece
> > of HW like this but if the usecase is for userspace to control the on/off states,
> > then I guess we would need .get() and .set(). Or some kind of "frontend" driver
> > making use of the consumer helpers.
> 
> There is already GPIO_V2_LINE_SET_CONFIG_IOCTL
> in <uapi/linux/gpio.h> so setting configs from userspace is no issue,
> just use the character device.
> 
> You will need to add I think two new config flags for userspace:
> GPIO_V2_LINE_FLAG_OUTPUT_ENABLE
> GPIO_V2_LINE_FLAG_OUTPUT_DISABLE
> 
> And update gpio_v2_line_config_flags_to_desc_flags() in
> drivers/gpio/gpiolib-cdev.c accordingly.
> 
> Then you probably want some tests or examples in libgpiod to make
> sure userspace is fine. Bartosz knows all about how to do this.
> 

It seems there's no need for userspace control. If you look at v3, it seems
we don't really need it to be a gpiochip (at least, I think). Maybe take a look,
you might have some good pointers :)


Thx!
- Nuno Sá
Re: [PATCH 2/2] gpio: adg1712: add driver support
Posted by Nuno Sá 3 months, 1 week ago
On Fri, 2025-10-31 at 16:07 +0000, Antoniu Miclaus wrote:
> Add driver support for the ADG1712, which contains four independent
> single-pole/single-throw (SPST) switches and operates with a
> low-voltage single supply range from +1.08V to +5.5V or a low-voltage
> dual supply range from ±1.08V to ±2.75V.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  drivers/gpio/Kconfig        |   9 +++
>  drivers/gpio/Makefile       |   1 +
>  drivers/gpio/gpio-adg1712.c | 146 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 156 insertions(+)
>  create mode 100644 drivers/gpio/gpio-adg1712.c
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 7ee3afbc2b05..3fac05823eae 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -157,6 +157,15 @@ config GPIO_74XX_MMIO
>  	    8 bits:	74244 (Input), 74273 (Output)
>  	    16 bits:	741624 (Input), 7416374 (Output)
>  
> +config GPIO_ADG1712
> +	tristate "Analog Devices ADG1712 quad SPST switch GPIO driver"
> +	depends on GPIOLIB
> +	help
> +	  GPIO driver for Analog Devices ADG1712 quad single-pole,
> +	  single-throw (SPST) switch. The driver provides a GPIO controller
> +	  interface where each GPIO line controls one of the four independent
> +	  analog switches on the ADG1712.
> +
>  config GPIO_ALTERA
>  	tristate "Altera GPIO"
>  	select GPIOLIB_IRQCHIP
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index ec296fa14bfd..9043d2d07a15 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_GPIO_104_IDI_48)		+= gpio-104-idi-48.o
>  obj-$(CONFIG_GPIO_104_IDIO_16)		+= gpio-104-idio-16.o
>  obj-$(CONFIG_GPIO_74X164)		+= gpio-74x164.o
>  obj-$(CONFIG_GPIO_74XX_MMIO)		+= gpio-74xx-mmio.o
> +obj-$(CONFIG_GPIO_ADG1712)		+= gpio-adg1712.o
>  obj-$(CONFIG_GPIO_ADNP)			+= gpio-adnp.o
>  obj-$(CONFIG_GPIO_ADP5520)		+= gpio-adp5520.o
>  obj-$(CONFIG_GPIO_ADP5585)		+= gpio-adp5585.o
> diff --git a/drivers/gpio/gpio-adg1712.c b/drivers/gpio/gpio-adg1712.c
> new file mode 100644
> index 000000000000..f8d3481ac9d0
> --- /dev/null
> +++ b/drivers/gpio/gpio-adg1712.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Analog Devices ADG1712 quad SPST switch GPIO driver
> + *
> + * Copyright 2025 Analog Devices Inc.
> + *
> + * Author: Antoniu Miclaus <antoniu.miclaus@analog.com>
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +
> +#define ADG1712_NUM_GPIOS	4
> +
> +struct adg1712 {
> +	struct gpio_chip chip;
> +	struct gpio_desc *switch_gpios[ADG1712_NUM_GPIOS];
> +};
> +
> +static int adg1712_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +	return GPIO_LINE_DIRECTION_OUT;
> +}
> +
> +static int adg1712_direction_input(struct gpio_chip *chip, unsigned int offset)
> +{
> +	return -EINVAL;
> +}

Did not checked gpiolib for this but do we need the above given that we always
return GPIO_LINE_DIRECTION_OUT?

> +
> +static int adg1712_direction_output(struct gpio_chip *chip, unsigned int offset,
> +				    int value)
> +{
> +	struct adg1712 *adg1712 = gpiochip_get_data(chip);
> +
> +	if (offset >= ADG1712_NUM_GPIOS)
> +		return -EINVAL;

I don't think above can happen.

> +
> +	gpiod_set_value_cansleep(adg1712->switch_gpios[offset], value);

return gpiod_set_value_cansleep().

> +	return 0;
> +}
> +
> +static int adg1712_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> +	struct adg1712 *adg1712 = gpiochip_get_data(chip);
> +
> +	if (offset >= ADG1712_NUM_GPIOS)
> +		return -EINVAL;

Ditto

> +
> +	gpiod_set_value_cansleep(adg1712->switch_gpios[offset], value);
> +	return 0;
> +}
> +
> +static int adg1712_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +	struct adg1712 *adg1712 = gpiochip_get_data(chip);
> +
> +	if (offset >= ADG1712_NUM_GPIOS)
> +		return -EINVAL;
> +
> +	return gpiod_get_value_cansleep(adg1712->switch_gpios[offset]);
> +}
> +
> +static int adg1712_set_multiple(struct gpio_chip *chip, unsigned long *mask,
> +				 unsigned long *bits)
> +{
> +	struct adg1712 *adg1712 = gpiochip_get_data(chip);
> +	int i;
> +
> +	for_each_set_bit(i, mask, ADG1712_NUM_GPIOS) {
> +		gpiod_set_value_cansleep(adg1712->switch_gpios[i],
> +					 test_bit(i, bits));

Error handling.

> +	}
> +
> +	return 0;
> +}
> +
> +static const struct gpio_chip adg1712_gpio_chip = {
> +	.label			= "adg1712",
> +	.owner			= THIS_MODULE,
> +	.get_direction		= adg1712_get_direction,
> +	.direction_input	= adg1712_direction_input,
> +	.direction_output	= adg1712_direction_output,
> +	.get			= adg1712_get,
> +	.set			= adg1712_set,
> +	.set_multiple		= adg1712_set_multiple,
> +	.base			= -1,
> +	.ngpio			= ADG1712_NUM_GPIOS,
> +	.can_sleep		= true,
> +};
> +
> +static int adg1712_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct adg1712 *adg1712;
> +	int ret, i;
> +	char gpio_name[16];
> +
> +	adg1712 = devm_kzalloc(dev, sizeof(*adg1712), GFP_KERNEL);
> +	if (!adg1712)
> +		return -ENOMEM;
> +
> +	adg1712->chip = adg1712_gpio_chip;
> +	adg1712->chip.parent = dev;
> +
> +	for (i = 0; i < ADG1712_NUM_GPIOS; i++) {
> +		snprintf(gpio_name, sizeof(gpio_name), "switch%d", i + 1);

Just a suggestion. Instead of the snprintf(), you could have a const array of
strings and just go over it. Not a big deal to me though. You could also
consider devm_gpiod_get_array()

> +		adg1712->switch_gpios[i] = devm_gpiod_get(dev, gpio_name,
> +							  GPIOD_OUT_LOW);

Should we make assumptions on the initial value? Not sure if GPIO_ASIS would
make sense here.

> +		if (IS_ERR(adg1712->switch_gpios[i]))
> +			return dev_err_probe(dev, PTR_ERR(adg1712->switch_gpios[i]),
> +					     "failed to get %s gpio\n", gpio_name);
> +	}
> +
> +	ret = devm_gpiochip_add_data(dev, &adg1712->chip, adg1712);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to add gpio chip\n");
> +
> +	dev_info(dev, "ADG1712 %u-GPIO expander registered\n",
> +		 adg1712->chip.ngpio);

Drop the above or turn it into dev_dbg()

- Nuno Sá
>