linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] gpio: Add support for ADG1712 quad SPST switch
@ 2025-10-31 16:07 Antoniu Miclaus
  2025-10-31 16:07 ` [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support Antoniu Miclaus
  2025-10-31 16:07 ` [PATCH 2/2] gpio: adg1712: add driver support Antoniu Miclaus
  0 siblings, 2 replies; 12+ messages in thread
From: Antoniu Miclaus @ 2025-10-31 16:07 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel
  Cc: 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support
  2025-10-31 16:07 [PATCH 0/2] gpio: Add support for ADG1712 quad SPST switch Antoniu Miclaus
@ 2025-10-31 16:07 ` Antoniu Miclaus
  2025-10-31 17:38   ` Rob Herring (Arm)
  2025-11-10 10:27   ` Linus Walleij
  2025-10-31 16:07 ` [PATCH 2/2] gpio: adg1712: add driver support Antoniu Miclaus
  1 sibling, 2 replies; 12+ messages in thread
From: Antoniu Miclaus @ 2025-10-31 16:07 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel
  Cc: 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] gpio: adg1712: add driver support
  2025-10-31 16:07 [PATCH 0/2] gpio: Add support for ADG1712 quad SPST switch Antoniu Miclaus
  2025-10-31 16:07 ` [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support Antoniu Miclaus
@ 2025-10-31 16:07 ` Antoniu Miclaus
  2025-11-03 11:18   ` Nuno Sá
  2025-11-10 10:30   ` Linus Walleij
  1 sibling, 2 replies; 12+ messages in thread
From: Antoniu Miclaus @ 2025-10-31 16:07 UTC (permalink / raw)
  To: Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel
  Cc: 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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support
  2025-10-31 16:07 ` [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support Antoniu Miclaus
@ 2025-10-31 17:38   ` Rob Herring (Arm)
  2025-11-10 10:27   ` Linus Walleij
  1 sibling, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2025-10-31 17:38 UTC (permalink / raw)
  To: Antoniu Miclaus
  Cc: linux-gpio, Linus Walleij, Krzysztof Kozlowski, Conor Dooley,
	devicetree, linux-kernel, Bartosz Golaszewski


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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-10-31 16:07 ` [PATCH 2/2] gpio: adg1712: add driver support Antoniu Miclaus
@ 2025-11-03 11:18   ` Nuno Sá
  2025-11-10 10:30   ` Linus Walleij
  1 sibling, 0 replies; 12+ messages in thread
From: Nuno Sá @ 2025-11-03 11:18 UTC (permalink / raw)
  To: Antoniu Miclaus, Linus Walleij, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support
  2025-10-31 16:07 ` [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support Antoniu Miclaus
  2025-10-31 17:38   ` Rob Herring (Arm)
@ 2025-11-10 10:27   ` Linus Walleij
  1 sibling, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2025-11-10 10:27 UTC (permalink / raw)
  To: Antoniu Miclaus, Peter Rosin
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-gpio, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-10-31 16:07 ` [PATCH 2/2] gpio: adg1712: add driver support Antoniu Miclaus
  2025-11-03 11:18   ` Nuno Sá
@ 2025-11-10 10:30   ` Linus Walleij
  2025-11-10 12:32     ` Nuno Sá
  1 sibling, 1 reply; 12+ messages in thread
From: Linus Walleij @ 2025-11-10 10:30 UTC (permalink / raw)
  To: Antoniu Miclaus
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-gpio, devicetree, linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-11-10 10:30   ` Linus Walleij
@ 2025-11-10 12:32     ` Nuno Sá
  2025-11-11 11:05       ` Linus Walleij
  0 siblings, 1 reply; 12+ messages in thread
From: Nuno Sá @ 2025-11-10 12:32 UTC (permalink / raw)
  To: Linus Walleij, Antoniu Miclaus
  Cc: Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-gpio, devicetree, linux-kernel

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á


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-11-10 12:32     ` Nuno Sá
@ 2025-11-11 11:05       ` Linus Walleij
  2025-11-11 16:01         ` Nuno Sá
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Walleij @ 2025-11-11 11:05 UTC (permalink / raw)
  To: Nuno Sá
  Cc: Antoniu Miclaus, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-11-11 11:05       ` Linus Walleij
@ 2025-11-11 16:01         ` Nuno Sá
  2025-11-18 22:54           ` Linus Walleij
  0 siblings, 1 reply; 12+ messages in thread
From: Nuno Sá @ 2025-11-11 16:01 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Antoniu Miclaus, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-11-11 16:01         ` Nuno Sá
@ 2025-11-18 22:54           ` Linus Walleij
  2025-11-19  9:38             ` Nuno Sá
  0 siblings, 1 reply; 12+ messages in thread
From: Linus Walleij @ 2025-11-18 22:54 UTC (permalink / raw)
  To: Nuno Sá
  Cc: Antoniu Miclaus, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] gpio: adg1712: add driver support
  2025-11-18 22:54           ` Linus Walleij
@ 2025-11-19  9:38             ` Nuno Sá
  0 siblings, 0 replies; 12+ messages in thread
From: Nuno Sá @ 2025-11-19  9:38 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Antoniu Miclaus, Bartosz Golaszewski, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
	linux-kernel

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á

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-11-19  9:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-31 16:07 [PATCH 0/2] gpio: Add support for ADG1712 quad SPST switch Antoniu Miclaus
2025-10-31 16:07 ` [PATCH 1/2] dt-bindings: gpio: adg1712: add adg1712 support Antoniu Miclaus
2025-10-31 17:38   ` Rob Herring (Arm)
2025-11-10 10:27   ` Linus Walleij
2025-10-31 16:07 ` [PATCH 2/2] gpio: adg1712: add driver support Antoniu Miclaus
2025-11-03 11:18   ` Nuno Sá
2025-11-10 10:30   ` Linus Walleij
2025-11-10 12:32     ` Nuno Sá
2025-11-11 11:05       ` Linus Walleij
2025-11-11 16:01         ` Nuno Sá
2025-11-18 22:54           ` Linus Walleij
2025-11-19  9:38             ` Nuno Sá

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).