[PATCH 0/2] gpio: Add support for ADG1712 quad SPST switch

[PATCH 0/2] gpio: Add support for ADG1712 quad SPST switch

[Antoniu Miclaus]

This patch series adds support for the Analog Devices ADG1712 quad
single-pole/single-throw (SPST) switch GPIO driver.

The ADG1712 contains four independent 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. Each switch is controlled by
a dedicated GPIO input pin.

The driver provides a GPIO controller interface where each GPIO line
controls one of the four independent analog switches on the ADG1712.
This allows software to dynamically control signal routing through
the analog switches.

Patch 1 adds the device tree bindings documentation.
Patch 2 adds the GPIO driver implementation.

Antoniu Miclaus (2):
  dt-bindings: gpio: adg1712: add adg1712 support
  gpio: adg1712: add driver support

 .../devicetree/bindings/gpio/adi,adg1712.yaml |  75 +++++++++
 drivers/gpio/Kconfig                          |   9 ++
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-adg1712.c                   | 146 ++++++++++++++++++
 4 files changed, 231 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/adi,adg1712.yaml
 create mode 100644 drivers/gpio/gpio-adg1712.c

-- 
2.43.0

[PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support

[Antoniu Miclaus]

Add devicetree bindings for adg1712 SPST quad switch.

Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
---
 .../devicetree/bindings/gpio/adi,adg1712.yaml | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/adi,adg1712.yaml

diff --git a/Documentation/devicetree/bindings/gpio/adi,adg1712.yaml b/Documentation/devicetree/bindings/gpio/adi,adg1712.yaml
new file mode 100644
index 000000000000..2c26d2a7def8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/adi,adg1712.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/adi,adg1712.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADG1712 quad SPST switch GPIO controller
+
+maintainers:
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+
+description: |
+  Bindings for Analog Devices ADG1712 quad single-pole, single-throw (SPST)
+  switch controlled by GPIOs. The device features four independent switches,
+  each controlled by a dedicated GPIO input pin.
+
+  Each GPIO line exposed by this controller corresponds to one of the four
+  switches (SW1-SW4) on the ADG1712. Setting a GPIO line high enables the
+  corresponding switch, while setting it low disables the switch.
+
+properties:
+  compatible:
+    const: adi,adg1712
+
+  switch1-gpios:
+    description: GPIO connected to the IN1 control pin (controls SW1)
+    maxItems: 1
+
+  switch2-gpios:
+    description: GPIO connected to the IN2 control pin (controls SW2)
+    maxItems: 1
+
+  switch3-gpios:
+    description: GPIO connected to the IN3 control pin (controls SW3)
+    maxItems: 1
+
+  switch4-gpios:
+    description: GPIO connected to the IN4 control pin (controls SW4)
+    maxItems: 1
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+    description: |
+      The first cell is the GPIO number (0-3 corresponding to SW1-SW4).
+      The second cell specifies GPIO flags.
+
+required:
+  - compatible
+  - switch1-gpios
+  - switch2-gpios
+  - switch3-gpios
+  - switch4-gpios
+  - gpio-controller
+  - "#gpio-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    adg1712: gpio-expander {
+        compatible = "adi,adg1712";
+        gpio-controller;
+        #gpio-cells = <2>;
+
+        switch1-gpios = <&gpio 10 GPIO_ACTIVE_HIGH>;
+        switch2-gpios = <&gpio 11 GPIO_ACTIVE_HIGH>;
+        switch3-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
+        switch4-gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
+    };
+
+...
\ No newline at end of file
-- 
2.43.0

Re: [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support

[Rob Herring (Arm)]

On Fri, 31 Oct 2025 16:07:04 +0000, Antoniu Miclaus wrote:
> Add devicetree bindings for adg1712 SPST quad switch.
> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> ---
>  .../devicetree/bindings/gpio/adi,adg1712.yaml | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/adi,adg1712.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/gpio/adi,adg1712.yaml:75:4: [error] no new line character at the end of file (new-line-at-end-of-file)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20251031160710.13343-2-antoniu.miclaus@analog.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support

[Linus Walleij]

Hi Antoniu,

thanks for your patch!

Add
Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adg1712.pdf

Before signed-off-by, thanks.

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

> +title: Analog Devices ADG1712 quad SPST switch GPIO controller
> +
> +maintainers:
> +  - Antoniu Miclaus <antoniu.miclaus@analog.com>
> +
> +description: |
> +  Bindings for Analog Devices ADG1712 quad single-pole, single-throw (SPST)
> +  switch controlled by GPIOs. The device features four independent switches,
> +  each controlled by a dedicated GPIO input pin.
> +
> +  Each GPIO line exposed by this controller corresponds to one of the four
> +  switches (SW1-SW4) on the ADG1712. Setting a GPIO line high enables the
> +  corresponding switch, while setting it low disables the switch.

There are two unclarities here:

- I know what an SPST switch is, but how is that electrically controlled?
  Is it actually a good old electro-magnetic relay? There are clearly
  details missing here. When I look in the datasheet, a symbol for a
  relay is present in the schematics. At least explain that they work
  "as a relay replacement" (literal wording from the datasheet) so
  we know what this is.

- GPIO is general purpose input/output. This is a narrow fit with that
  concept. This device is more of a general purpose mechanical
  current switch. We need some motivation here, explaining why
  GPIO is a good, operating system-neutral description of what this
  device does.

Perhaps we need to create a new binding category
dt-bindings/switch for this, even if in Linux specifically we chose
to model this as a GPIO, it could just be something we do in
Linux, Zephyr for example might want to have a dedicated driver
for switches.

Also I would like Peter Rosin's eye on this, as we have
dt-bindings/mux which is selecting one analog line out of many
and it's close enough.

> +  switch1-gpios:
> +    description: GPIO connected to the IN1 control pin (controls SW1)
> +    maxItems: 1
> +
> +  switch2-gpios:
> +    description: GPIO connected to the IN2 control pin (controls SW2)
> +    maxItems: 1
> +
> +  switch3-gpios:
> +    description: GPIO connected to the IN3 control pin (controls SW3)
> +    maxItems: 1
> +
> +  switch4-gpios:
> +    description: GPIO connected to the IN4 control pin (controls SW4)
> +    maxItems: 1

Why not just use an array of GPIOs? The property has the suffix "gpios"
(pluralis) after all.

I'd just use switch-gpios = <1, 2, 3, 4>...

> +  gpio-controller: true

So this switching capacity expose four new GPIOs, are these really
GPIOs, that's the question. I think we might need a new binding
category. Either this is switch, GPIO or some type of amplifier.

Yours,
Linus Walleij

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

[Antoniu Miclaus]

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

[Nuno Sá]

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á
> 

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

[Linus Walleij]

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

[Nuno Sá]

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

[Linus Walleij]

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

[Nuno Sá]

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

[Linus Walleij]

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

[Nuno Sá]

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á