[PATCH v2 0/2] Add support for ADG1414 Serially-Controlled Octal SPST Switches

[PATCH v2 0/2] Add support for ADG1414 Serially-Controlled Octal SPST Switches

[Kim Seer Paller]

Apologies for the long delay in following up on this patch series.
I checked the gpio-regmap abstraction to achieve a smaller footprint,
but found that it may not suitable for the ADG1414. The ADG1414 device
requires direct manipulation of individual bits to control the switches.
In the regmap_config, reg_bits and val_bits need to be set to reflect a
byte for instruction and a byte for data. However, the ADG1414 device
directly changes the bit itself (e.g., bit 0 to change on/off switch 0).
I think the standard regmap read/write API may not be applicable.
Additionally, the size of the transaction dynamically changes based on
the number of daisy-chained devices.

This version address the feedback provided and includes the necessary
improvements.

ADG1414:
* Define a static struct regmap_bus to wrap custom read/write functions.
* Use devm_mutex_init() to initialize the mutex.

Bindings:
* Modify filename to adi,adg1414-gpio.
* Add type definition for #daisy-chained-devices property.
* Modify title and description to describe hardware.

- Kim Seer Paller

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
Kim Seer Paller (2):
      dt-bindings: gpio: add adg1414
      gpio: gpio-adg1414: New driver

 .../devicetree/bindings/gpio/adi,adg1414-gpio.yaml |  68 +++++++++
 MAINTAINERS                                        |   7 +
 drivers/gpio/Kconfig                               |  10 ++
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-adg1414.c                        | 162 +++++++++++++++++++++
 5 files changed, 248 insertions(+)
---
base-commit: 4dc1d1bec89864d8076e5ab314f86f46442bfb02
change-id: 20250213-for_upstream-a490d1a04bf2

Best regards,
-- 
Kim Seer Paller <kimseer.paller@analog.com>

[PATCH v2 1/2] dt-bindings: gpio: add adg1414

[Kim Seer Paller]

The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS Serially-Controlled
Octal SPST Switches.

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 .../devicetree/bindings/gpio/adi,adg1414-gpio.yaml | 68 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++
 2 files changed, 74 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/adi,adg1414-gpio.yaml b/Documentation/devicetree/bindings/gpio/adi,adg1414-gpio.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..2c91175d3c7e0a030a894953abfad003ace23744
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/adi,adg1414-gpio.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/adi,adg1414-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADG1414 Serially-Controlled Octal SPST Switches
+
+maintainers:
+  - Kim Seer Paller <kimseer.paller@analog.com>
+
+description:
+  The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS serially-controlled
+  octal SPST switches.
+
+properties:
+  compatible:
+    enum:
+      - adi,adg14140-gpio
+
+  reg:
+    maxItems: 1
+
+  gpio-controller: true
+
+  '#gpio-cells':
+    const: 2
+
+  spi-cpha: true
+
+  reset-gpios:
+    description: RESET/Logic Power Supply Input (VL). When the RESET/VL pin is
+      low, all switches are off and the appropriate registers are cleared to 0.
+    maxItems: 1
+
+  '#daisy-chained-devices':
+    description: The number of daisy-chained devices.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 1
+    minimum: 1
+    maximum: 4
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        gpio@0 {
+            compatible = "adi,adg14140-gpio";
+            reg = <0>;
+            spi-max-frequency = <1000000>;
+            spi-cpha;
+            reset-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 25c86f47353de25c88291cc7fd6c4e9bfb12d5c4..66d92be0f57daa9eabb48d7e53b6b2bea0c40863 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -498,6 +498,12 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/net/ieee802154/adf7242.txt
 F:	drivers/net/ieee802154/adf7242.c
 
+ADG1414 SPST Switch Driver
+M:	Kim Seer Paller <kimseer.paller@analog.com>
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
+
 ADM1025 HARDWARE MONITOR DRIVER
 M:	Jean Delvare <jdelvare@suse.com>
 L:	linux-hwmon@vger.kernel.org

-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: gpio: add adg1414

[kernel test robot]

Hi Kim,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4dc1d1bec89864d8076e5ab314f86f46442bfb02]

url:    https://github.com/intel-lab-lkp/linux/commits/Kim-Seer-Paller/dt-bindings-gpio-add-adg1414/20250213-211900
base:   4dc1d1bec89864d8076e5ab314f86f46442bfb02
patch link:    https://lore.kernel.org/r/20250213-for_upstream-v2-1-ec4eff3b3cd5%40analog.com
patch subject: [PATCH v2 1/2] dt-bindings: gpio: add adg1414
reproduce: (https://download.01.org/0day-ci/archive/20250213/202502132314.mPTyC8ew-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502132314.mPTyC8ew-lkp@intel.com/

All warnings (new ones prefixed by >>):

   Warning: Documentation/translations/ja_JP/SubmittingPatches references a file that doesn't exist: linux-2.6.12-vanilla/Documentation/dontdiff
   Warning: Documentation/translations/zh_CN/admin-guide/README.rst references a file that doesn't exist: Documentation/dev-tools/kgdb.rst
   Warning: Documentation/translations/zh_CN/dev-tools/gdb-kernel-debugging.rst references a file that doesn't exist: Documentation/dev-tools/gdb-kernel-debugging.rst
   Warning: Documentation/translations/zh_TW/admin-guide/README.rst references a file that doesn't exist: Documentation/dev-tools/kgdb.rst
   Warning: Documentation/translations/zh_TW/dev-tools/gdb-kernel-debugging.rst references a file that doesn't exist: Documentation/dev-tools/gdb-kernel-debugging.rst
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
   Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/leds/backlight/ti,lp8864.yaml
   Warning: lib/Kconfig.debug references a file that doesn't exist: Documentation/dev-tools/fault-injection/fault-injection.rst
   Using alabaster theme

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] dt-bindings: gpio: add adg1414

[Krzysztof Kozlowski]

On 13/02/2025 14:15, Kim Seer Paller wrote:
> +maintainers:
> +  - Kim Seer Paller <kimseer.paller@analog.com>
> +
> +description:
> +  The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS serially-controlled
> +  octal SPST switches.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,adg14140-gpio


Is ADG1414 anything else than GPIO? Where are the rest of the bindings then?

> +
> +  reg:
> +    maxItems: 1
> +


...

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 25c86f47353de25c88291cc7fd6c4e9bfb12d5c4..66d92be0f57daa9eabb48d7e53b6b2bea0c40863 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -498,6 +498,12 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/net/ieee802154/adf7242.txt
>  F:	drivers/net/ieee802154/adf7242.c
>  
> +ADG1414 SPST Switch Driver
> +M:	Kim Seer Paller <kimseer.paller@analog.com>
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
As reported - wrong path.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: gpio: add adg1414

[Linus Walleij]

On Thu, Feb 13, 2025 at 7:16 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 13/02/2025 14:15, Kim Seer Paller wrote:
> > +maintainers:
> > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > +
> > +description:
> > +  The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS serially-controlled
> > +  octal SPST switches.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,adg14140-gpio
>
>
> Is ADG1414 anything else than GPIO? Where are the rest of the bindings then?

I read the spec and it is actually an SPI-controlled switch.

(As in "power switch", not "network switch".)

It's a bit interesting since we have no "switch" subsystem, but there
is "mux".

The question is whether this should be considered some kind of
"gpio" (due to the nature of switches being off/on) in order to not
complicate our world too much or if we need to create a whole
new device class for switches.

Yours,
Linus Walleij

[PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Kim Seer Paller]

The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS Serially-Controlled
Octal SPST Switches

Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
---
 MAINTAINERS                 |   1 +
 drivers/gpio/Kconfig        |  10 +++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-adg1414.c | 162 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 174 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 66d92be0f57daa9eabb48d7e53b6b2bea0c40863..65877ca70b7e929353798c5639bf38593241d83e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -503,6 +503,7 @@ M:	Kim Seer Paller <kimseer.paller@analog.com>
 S:	Supported
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
+F:	drivers/gpio/gpio-adg1414.c
 
 ADM1025 HARDWARE MONITOR DRIVER
 M:	Jean Delvare <jdelvare@suse.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 98b4d1633b258b93d05ba66d53f647e7ec6ac364..4b797ddebd21bc879524f8504b343add2730c793 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1797,6 +1797,16 @@ config GPIO_74X164
 	  shift registers. This driver can be used to provide access
 	  to more GPIO outputs.
 
+config GPIO_ADG1414
+	tristate "ADG1414 Serially-Controlled Octal SPST Switches"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices ADG1414
+	  Serially-Controlled Octal SPST Switches
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called gpio-adg1414.
+
 config GPIO_MAX3191X
 	tristate "Maxim MAX3191x industrial serializer"
 	select CRC8
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index af3ba4d81b583842893ea69e677fbe2abf31bc7b..58d723cb32feebd35db13ae9cae5d232feba3f5b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -24,6 +24,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_ADG1414)		+= gpio-adg1414.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-adg1414.c b/drivers/gpio/gpio-adg1414.c
new file mode 100644
index 0000000000000000000000000000000000000000..91e07c38fb1c86162afd9cfcdf1d7cfcccb03234
--- /dev/null
+++ b/drivers/gpio/gpio-adg1414.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ADG1414 Serially-Controlled Octal SPST Switches
+ *
+ * Copyright 2025 Analog Devices Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#define ADG1414_MAX_DEVICES		4
+
+struct adg1414_state {
+	struct spi_device *spi;
+	struct gpio_chip chip;
+	struct regmap *regmap;
+	struct mutex lock; /* protect sensor state */
+	u32 buf;
+
+	__be32 tx __aligned(ARCH_DMA_MINALIGN);
+};
+
+static int adg1414_spi_write(void *context, const void *data, size_t count)
+{
+	struct adg1414_state *st = context;
+
+	struct spi_transfer xfer = {
+		.tx_buf = &st->tx,
+		.len = count,
+	};
+
+	return spi_sync_transfer(st->spi, &xfer, 1);
+}
+
+static int adg1414_spi_read(void *context, const void *reg, size_t reg_size,
+			    void *val, size_t val_size)
+{
+	return 0;
+}
+
+static int adg1414_get(struct gpio_chip *chip, unsigned int offset)
+{
+	struct adg1414_state *st = gpiochip_get_data(chip);
+
+	guard(mutex)(&st->lock);
+
+	return st->buf & BIT(offset);
+}
+
+static void adg1414_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	struct adg1414_state *st = gpiochip_get_data(chip);
+
+	guard(mutex)(&st->lock);
+
+	if (value)
+		st->buf |= BIT(offset);
+	else
+		st->buf &= ~BIT(offset);
+
+	st->tx = cpu_to_be32(st->buf << (32 - st->chip.ngpio));
+
+	adg1414_spi_write(st, 0, st->chip.ngpio / 8);
+}
+
+static const struct regmap_bus adg1414_regmap_bus = {
+	.write = adg1414_spi_write,
+	.read =	adg1414_spi_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_BIG,
+	.val_format_endian_default = REGMAP_ENDIAN_BIG,
+};
+
+static const struct regmap_config adg1414_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+};
+
+static int adg1414_get_direction(struct gpio_chip *chip,
+				 unsigned int offset)
+{
+	return GPIO_LINE_DIRECTION_OUT;
+}
+
+static int adg1414_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct adg1414_state *st;
+	struct gpio_desc *reset;
+	u32 num_devices;
+	int ret;
+
+	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return -ENOMEM;
+
+	st->spi = spi;
+
+	st->regmap = devm_regmap_init(dev, &adg1414_regmap_bus, st,
+				      &adg1414_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to initialize regmap");
+
+	/* Use reset pin to reset the device */
+	reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset))
+		return dev_err_probe(dev, PTR_ERR(reset),
+				     "Failed to get reset gpio");
+
+	if (reset) {
+		fsleep(1);
+		gpiod_set_value_cansleep(reset, 0);
+	}
+
+	num_devices = 1;
+	ret = device_property_read_u32(dev, "#daisy-chained-devices",
+				       &num_devices);
+	if (!ret) {
+		if (!num_devices || num_devices > ADG1414_MAX_DEVICES)
+			return dev_err_probe(dev, ret,
+			       "Failed to get daisy-chained-devices property\n");
+	}
+
+	st->chip.label = "adg1414";
+	st->chip.parent = dev;
+	st->chip.get_direction = adg1414_get_direction;
+	st->chip.set = adg1414_set;
+	st->chip.get = adg1414_get;
+	st->chip.base = -1;
+	st->chip.ngpio =  num_devices * 8;
+	st->chip.can_sleep = true;
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	return devm_gpiochip_add_data(dev, &st->chip, st);
+}
+
+static const struct of_device_id adg1414_of_match[] = {
+	{ .compatible = "adi,adg1414-gpio" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, adg1414_of_match);
+
+static struct spi_driver adg1414_driver = {
+	.driver = {
+		.name = "adg1414-gpio",
+		.of_match_table = adg1414_of_match,
+	},
+	.probe = adg1414_probe,
+};
+module_spi_driver(adg1414_driver);
+
+MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>");
+MODULE_DESCRIPTION("ADG1414 Serially-Controlled Octal SPST Switches");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Linus Walleij]

Hi Kim,

thanks for your patch!

On Thu, Feb 13, 2025 at 2:17 PM Kim Seer Paller
<kimseer.paller@analog.com> wrote:

> The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS Serially-Controlled
> Octal SPST Switches
>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>

OK so I looked at the data sheet and it looks like this:

A  o-------/ --------o B

It'a a switch.

Why is this switch a "gpio", other than that it is convenient
to use the GPIO abstraction to control it?

GPIO is usually devices that can drive a line high or low.
This is very far from that. This could switch some analog
line or whatever, right?

Now, the kernel does not have switch subsystem I think,
so this is something like a special case, so we might be
compelled to make an exception, if the users will all be in
say userspace and make use of this switch for factory lines
or similar.

Otherwise there is also drivers/mux, but maybe a 1:1
mux is an odd type of switch...

> +static int adg1414_spi_write(void *context, const void *data, size_t count)
> +{
> +       struct adg1414_state *st = context;
> +
> +       struct spi_transfer xfer = {
> +               .tx_buf = &st->tx,
> +               .len = count,
> +       };
> +
> +       return spi_sync_transfer(st->spi, &xfer, 1);
> +}
> +
> +static int adg1414_spi_read(void *context, const void *reg, size_t reg_size,
> +                           void *val, size_t val_size)
> +{
> +       return 0;
> +}
> +
> +static int adg1414_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +       struct adg1414_state *st = gpiochip_get_data(chip);
> +
> +       guard(mutex)(&st->lock);
> +
> +       return st->buf & BIT(offset);
> +}
> +
> +static void adg1414_set(struct gpio_chip *chip, unsigned int offset, int value)
> +{
> +       struct adg1414_state *st = gpiochip_get_data(chip);
> +
> +       guard(mutex)(&st->lock);
> +
> +       if (value)
> +               st->buf |= BIT(offset);
> +       else
> +               st->buf &= ~BIT(offset);
> +
> +       st->tx = cpu_to_be32(st->buf << (32 - st->chip.ngpio));
> +
> +       adg1414_spi_write(st, 0, st->chip.ngpio / 8);
> +}
> +
> +static const struct regmap_bus adg1414_regmap_bus = {
> +       .write = adg1414_spi_write,
> +       .read = adg1414_spi_read,
> +       .reg_format_endian_default = REGMAP_ENDIAN_BIG,
> +       .val_format_endian_default = REGMAP_ENDIAN_BIG,
> +};
> +
> +static const struct regmap_config adg1414_regmap_config = {
> +       .reg_bits = 8,
> +       .val_bits = 8,
> +};

This is not how regmap works. Your get/set callbacks for GPIO
should call regmap_get() and regmap_update_bits() to get/set
the individual bits.

So the adg1414_spi_write() needs to be done from inside the
regmap abstraction.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Nuno Sá]

Hi Linus,

On Fri, 2025-02-14 at 00:25 +0100, Linus Walleij wrote:
> Hi Kim,
> 
> thanks for your patch!
> 
> On Thu, Feb 13, 2025 at 2:17 PM Kim Seer Paller
> <kimseer.paller@analog.com> wrote:
> 
> > The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS Serially-Controlled
> > Octal SPST Switches
> > 
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> 
> OK so I looked at the data sheet and it looks like this:
> 
> A  o-------/ --------o B
> 
> It'a a switch.
> 
> Why is this switch a "gpio", other than that it is convenient
> to use the GPIO abstraction to control it?
> 
> GPIO is usually devices that can drive a line high or low.
> This is very far from that. This could switch some analog
> line or whatever, right?

I would say so yes but Kim should know better...
> 
> Now, the kernel does not have switch subsystem I think,
> so this is something like a special case, so we might be
> compelled to make an exception, if the users will all be in

Exactly, since we could not find anything, the best fit seemed like the gpio
subsystem. I was the one suggesting it since a new subsystem for a simple device
like this looked excessive. If we had more devices that would fit such a class
of devices, maybe it would make more sense to start thinking on such a
subsystem?

> say userspace and make use of this switch for factory lines
> or similar.

Kim should know better again (about usecases) but I would also assume this is
for userspace use.

Thanks!
- Nuno Sá

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Linus Walleij]

Let's check with Jonathan Cameron (IIO maintainer) on this as well.
He might have ideas.

For reference, the datasheet:
https://www.analog.com/media/en/technical-documentation/data-sheets/adg1414.pdf

(By the way: add the datasheet to a special Datasheet: tag in the
commit please!)

On Fri, Feb 14, 2025 at 2:17 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> On Fri, 2025-02-14 at 00:25 +0100, Linus Walleij wrote:

> > Now, the kernel does not have switch subsystem I think,
> > so this is something like a special case, so we might be
> > compelled to make an exception, if the users will all be in
>
> Exactly, since we could not find anything, the best fit seemed like the gpio
> subsystem. I was the one suggesting it since a new subsystem for a simple device
> like this looked excessive. If we had more devices that would fit such a class
> of devices, maybe it would make more sense to start thinking on such a
> subsystem?
>
> > say userspace and make use of this switch for factory lines
> > or similar.
>
> Kim should know better again (about usecases) but I would also assume this is
> for userspace use.

Actually the GPIO documentation Documentation/driver-api/gpio/using-gpio.rst
even talks about this for userspace use cases:

"The userspace ABI is intended for one-off deployments. Examples are prototypes,
factory lines, maker community projects, workshop specimen, production tools,
industrial automation, PLC-type use cases, door controllers, in short a piece
of specialized equipment that is not produced by the numbers, requiring
operators to have a deep knowledge of the equipment and knows about the
software-hardware interface to be set up. They should not have a natural fit
to any existing kernel subsystem and not be a good fit for an operating system,
because of not being reusable or abstract enough, or involving a lot of non
computer hardware related policy."

If this is the usecase, like controlling an external switch for such things,
using the GPIO subsystem might actually be reasonable in my opinion,
(even if the DT bindings end up in their own category).

If the switches control stuff related to computer machinery (i.e. integrated
into a laptop to switch on/off the fans...) then no. So it depends on how
and where it will be used.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Jonathan Cameron]

On Sat, 15 Feb 2025 00:22:20 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> Let's check with Jonathan Cameron (IIO maintainer) on this as well.
> He might have ideas.
> 
> For reference, the datasheet:
> https://www.analog.com/media/en/technical-documentation/data-sheets/adg1414.pdf
> 
> (By the way: add the datasheet to a special Datasheet: tag in the
> commit please!)
> 
> On Fri, Feb 14, 2025 at 2:17 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> > On Fri, 2025-02-14 at 00:25 +0100, Linus Walleij wrote:  
> 
> > > Now, the kernel does not have switch subsystem I think,
> > > so this is something like a special case, so we might be
> > > compelled to make an exception, if the users will all be in  
> >
> > Exactly, since we could not find anything, the best fit seemed like the gpio
> > subsystem. I was the one suggesting it since a new subsystem for a simple device
> > like this looked excessive. If we had more devices that would fit such a class
> > of devices, maybe it would make more sense to start thinking on such a
> > subsystem?
> >  
> > > say userspace and make use of this switch for factory lines
> > > or similar.  
> >
> > Kim should know better again (about usecases) but I would also assume this is
> > for userspace use.  
> 
> Actually the GPIO documentation Documentation/driver-api/gpio/using-gpio.rst
> even talks about this for userspace use cases:
> 
> "The userspace ABI is intended for one-off deployments. Examples are prototypes,
> factory lines, maker community projects, workshop specimen, production tools,
> industrial automation, PLC-type use cases, door controllers, in short a piece
> of specialized equipment that is not produced by the numbers, requiring
> operators to have a deep knowledge of the equipment and knows about the
> software-hardware interface to be set up. They should not have a natural fit
> to any existing kernel subsystem and not be a good fit for an operating system,
> because of not being reusable or abstract enough, or involving a lot of non
> computer hardware related policy."
> 
> If this is the usecase, like controlling an external switch for such things,
> using the GPIO subsystem might actually be reasonable in my opinion,
> (even if the DT bindings end up in their own category).
> 
> If the switches control stuff related to computer machinery (i.e. integrated
> into a laptop to switch on/off the fans...) then no. So it depends on how
> and where it will be used.

Maybe, treat them as a weird mux? A switch is similar to a mux with only
one connected path.  +CC Peter. 

Jonathan

> 
> Yours,
> Linus Walleij

RE: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Paller, Kim Seer]

> -----Original Message-----
> From: Linus Walleij <linus.walleij@linaro.org>
> Sent: Saturday, February 15, 2025 7:22 AM
> To: Nuno Sá <noname.nuno@gmail.com>; Jonathan Cameron
> <jic23@kernel.org>
> Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; Bartosz Golaszewski
> <brgl@bgdev.pl>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-iio <linux-iio@vger.kernel.org>
> Subject: Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver
> 
> [External]
> 
> Let's check with Jonathan Cameron (IIO maintainer) on this as well.
> He might have ideas.
> 
> For reference, the datasheet:
> https://www.analog.com/media/en/technical-documentation/data-
> sheets/adg1414.pdf
> 
> (By the way: add the datasheet to a special Datasheet: tag in the
> commit please!)
> 
> On Fri, Feb 14, 2025 at 2:17 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> > On Fri, 2025-02-14 at 00:25 +0100, Linus Walleij wrote:
> 
> > > Now, the kernel does not have switch subsystem I think,
> > > so this is something like a special case, so we might be
> > > compelled to make an exception, if the users will all be in
> >
> > Exactly, since we could not find anything, the best fit seemed like the gpio
> > subsystem. I was the one suggesting it since a new subsystem for a simple
> device
> > like this looked excessive. If we had more devices that would fit such a class
> > of devices, maybe it would make more sense to start thinking on such a
> > subsystem?
> >
> > > say userspace and make use of this switch for factory lines
> > > or similar.
> >
> > Kim should know better again (about usecases) but I would also assume this
> is
> > for userspace use.
> 
> Actually the GPIO documentation Documentation/driver-api/gpio/using-
> gpio.rst
> even talks about this for userspace use cases:
> 
> "The userspace ABI is intended for one-off deployments. Examples are
> prototypes,
> factory lines, maker community projects, workshop specimen, production tools,
> industrial automation, PLC-type use cases, door controllers, in short a piece
> of specialized equipment that is not produced by the numbers, requiring
> operators to have a deep knowledge of the equipment and knows about the
> software-hardware interface to be set up. They should not have a natural fit
> to any existing kernel subsystem and not be a good fit for an operating system,
> because of not being reusable or abstract enough, or involving a lot of non
> computer hardware related policy."
> 
> If this is the usecase, like controlling an external switch for such things,
> using the GPIO subsystem might actually be reasonable in my opinion,
> (even if the DT bindings end up in their own category).
> 
> If the switches control stuff related to computer machinery (i.e. integrated
> into a laptop to switch on/off the fans...) then no. So it depends on how
> and where it will be used.

In my case, this is a userspace use case. The ADG1414 was used to control the
ADMFM2000 Microwave Downconverter device. According to the ADMFM2000
datasheet, it requires control over 14 digital pins, which can be set high or low [1].
While these pins could be directly controlled using GPIO, the evaluation board for
the ADMFM2000 is designed to use the ADG1414 switch for this purpose [2].
ADG1414 is an SPI controlled switch that allows switching of these digital control lines.

Then, I have a custom userspace application to access sysfs, which in turn controls the
ADG1414, providing specific functions for the ADMFM2000. Using the GPIO subsystem
to interface with the ADG1414 perhaps a practical way to manage these control
signals.

In my opinion, using the GPIO subsystem for the ADG1414 may be a better option
given the intended use case. However, what do you suggest on how to proceed?

[1] https://www.analog.com/media/en/technical-documentation/data-sheets/admfm2000.pdf
[2] https://www.analog.com/en/resources/evaluation-hardware-and-software/evaluation-boards-kits/eval-admfm2000.html#eb-overview

All the best,
Kim

> 
> Yours,
> Linus Walleij

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Nuno Sá]

On Mon, 2025-02-17 at 07:02 +0000, Paller, Kim Seer wrote:
> 
> 
> > -----Original Message-----
> > From: Linus Walleij <linus.walleij@linaro.org>
> > Sent: Saturday, February 15, 2025 7:22 AM
> > To: Nuno Sá <noname.nuno@gmail.com>; Jonathan Cameron
> > <jic23@kernel.org>
> > Cc: Paller, Kim Seer <KimSeer.Paller@analog.com>; Bartosz Golaszewski
> > <brgl@bgdev.pl>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> > gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-iio <linux-iio@vger.kernel.org>
> > Subject: Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver
> > 
> > [External]
> > 
> > Let's check with Jonathan Cameron (IIO maintainer) on this as well.
> > He might have ideas.
> > 
> > For reference, the datasheet:
> > https://www.analog.com/media/en/technical-documentation/data-
> > sheets/adg1414.pdf
> > 
> > (By the way: add the datasheet to a special Datasheet: tag in the
> > commit please!)
> > 
> > On Fri, Feb 14, 2025 at 2:17 PM Nuno Sá <noname.nuno@gmail.com> wrote:
> > > On Fri, 2025-02-14 at 00:25 +0100, Linus Walleij wrote:
> > 
> > > > Now, the kernel does not have switch subsystem I think,
> > > > so this is something like a special case, so we might be
> > > > compelled to make an exception, if the users will all be in
> > > 
> > > Exactly, since we could not find anything, the best fit seemed like the
> > > gpio
> > > subsystem. I was the one suggesting it since a new subsystem for a simple
> > device
> > > like this looked excessive. If we had more devices that would fit such a
> > > class
> > > of devices, maybe it would make more sense to start thinking on such a
> > > subsystem?
> > > 
> > > > say userspace and make use of this switch for factory lines
> > > > or similar.
> > > 
> > > Kim should know better again (about usecases) but I would also assume this
> > is
> > > for userspace use.
> > 
> > Actually the GPIO documentation Documentation/driver-api/gpio/using-
> > gpio.rst
> > even talks about this for userspace use cases:
> > 
> > "The userspace ABI is intended for one-off deployments. Examples are
> > prototypes,
> > factory lines, maker community projects, workshop specimen, production
> > tools,
> > industrial automation, PLC-type use cases, door controllers, in short a
> > piece
> > of specialized equipment that is not produced by the numbers, requiring
> > operators to have a deep knowledge of the equipment and knows about the
> > software-hardware interface to be set up. They should not have a natural fit
> > to any existing kernel subsystem and not be a good fit for an operating
> > system,
> > because of not being reusable or abstract enough, or involving a lot of non
> > computer hardware related policy."
> > 
> > If this is the usecase, like controlling an external switch for such things,
> > using the GPIO subsystem might actually be reasonable in my opinion,
> > (even if the DT bindings end up in their own category).
> > 
> > If the switches control stuff related to computer machinery (i.e. integrated
> > into a laptop to switch on/off the fans...) then no. So it depends on how
> > and where it will be used.
> 
> In my case, this is a userspace use case. The ADG1414 was used to control the
> ADMFM2000 Microwave Downconverter device. According to the ADMFM2000
> datasheet, it requires control over 14 digital pins, which can be set high or
> low [1].
> While these pins could be directly controlled using GPIO, the evaluation board
> for
> the ADMFM2000 is designed to use the ADG1414 switch for this purpose [2].
> ADG1414 is an SPI controlled switch that allows switching of these digital
> control lines.
> 

AFAICT the mux subsystem does not have any userspace so it would already not fit
the above usecase. We could add a simple setter sysfs interface if Peter thinks
this belongs to the mux subsystem...

Let's see what Peter has to say about this.

What about misc devices? I mean, if there's no agreement... 

- Nuno Sá

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[Linus Walleij]

On Mon, Feb 17, 2025 at 8:02 AM Paller, Kim Seer
<KimSeer.Paller@analog.com> wrote:

> In my opinion, using the GPIO subsystem for the ADG1414 may be a better option
> given the intended use case. However, what do you suggest on how to proceed?

I think it's best to proceed with a GPIO driver unless someone has
better ideas.

Address my comments on the regmap use please!

Yours,
Linus Walleij

Re: [PATCH v2 2/2] gpio: gpio-adg1414: New driver

[kernel test robot]

Hi Kim,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4dc1d1bec89864d8076e5ab314f86f46442bfb02]

url:    https://github.com/intel-lab-lkp/linux/commits/Kim-Seer-Paller/dt-bindings-gpio-add-adg1414/20250213-211900
base:   4dc1d1bec89864d8076e5ab314f86f46442bfb02
patch link:    https://lore.kernel.org/r/20250213-for_upstream-v2-2-ec4eff3b3cd5%40analog.com
patch subject: [PATCH v2 2/2] gpio: gpio-adg1414: New driver
config: alpha-randconfig-r132-20250215 (https://download.01.org/0day-ci/archive/20250215/202502150933.vtaQAJRw-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 14.2.0
reproduce: (https://download.01.org/0day-ci/archive/20250215/202502150933.vtaQAJRw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502150933.vtaQAJRw-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/gpio/gpio-adg1414.c:68:31: sparse: sparse: Using plain integer as NULL pointer
   drivers/gpio/gpio-adg1414.c: note: in included file (through include/linux/smp.h, include/linux/lockdep.h, include/linux/spinlock.h, ...):
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true

vim +68 drivers/gpio/gpio-adg1414.c

    54	
    55	static void adg1414_set(struct gpio_chip *chip, unsigned int offset, int value)
    56	{
    57		struct adg1414_state *st = gpiochip_get_data(chip);
    58	
    59		guard(mutex)(&st->lock);
    60	
    61		if (value)
    62			st->buf |= BIT(offset);
    63		else
    64			st->buf &= ~BIT(offset);
    65	
    66		st->tx = cpu_to_be32(st->buf << (32 - st->chip.ngpio));
    67	
  > 68		adg1414_spi_write(st, 0, st->chip.ngpio / 8);
    69	}
    70	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki