I'm sending this patch for review, but I'm uncertain about the suitable subsystem
for the driver. Currently, it seems like the GPIO subsystem is the best fit.
If that's the case, it's open for review, and I appreciate any insights and feedback.
Best regards,
Kim Seer Paller
Kim Seer Paller (2):
dt-bindings: gpio: add adg1414
gpio: gpio-adg1414: New driver
.../bindings/gpio/gpio-adg1414.yaml | 66 ++++++++
MAINTAINERS | 7 +
drivers/gpio/Kconfig | 10 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-adg1414.c | 141 ++++++++++++++++++
5 files changed, 225 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
create mode 100644 drivers/gpio/gpio-adg1414.c
base-commit: 7a396820222d6d4c02057f41658b162bdcdadd0e
--
2.34.1
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 <[email protected]>
---
.../bindings/gpio/gpio-adg1414.yaml | 66 +++++++++++++++++++
MAINTAINERS | 6 ++
2 files changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
diff --git a/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
new file mode 100644
index 000000000..24a51e79f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/gpio-adg1414.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADG1414 SPST Switch Driver
+
+maintainers:
+ - Kim Seer Paller <[email protected]>
+
+description:
+ The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS serially-controlled
+ octal SPST switches.
+
+properties:
+ compatible:
+ enum:
+ - adi,adg1414
+
+ reg:
+ maxItems: 1
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ spi-cpha: true
+
+ reset-gpios:
+ description: GPIO specifier that resets the device.
+ maxItems: 1
+
+ '#daisy-chained-devices':
+ description: The number of daisy-chained devices.
+ 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,adg1414";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpha;
+ reset-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6..526145e69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -461,6 +461,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 <[email protected]>
+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 <[email protected]>
L: [email protected]
--
2.34.1
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 <[email protected]>
---
MAINTAINERS | 1 +
drivers/gpio/Kconfig | 10 +++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-adg1414.c | 141 ++++++++++++++++++++++++++++++++++++
4 files changed, 153 insertions(+)
create mode 100644 drivers/gpio/gpio-adg1414.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 526145e69..254ba7ea5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -466,6 +466,7 @@ M: Kim Seer Paller <[email protected]>
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 <[email protected]>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1301cec94..25d467d70 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1728,6 +1728,16 @@ config GPIO_74X164
shift registers. This driver can be used to provide access
to more GPIO outputs.
+config GPIO_ADG1414
+ tristate "ADG1414 SPST Switch Driver"
+ depends on SPI
+ help
+ Say yes here to build support for Analog Devices ADG1414 SPST
+ Switch Driver
+
+ 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 9e40af196..9ab45d128 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_AGGREGATOR) += gpio-aggregator.o
diff --git a/drivers/gpio/gpio-adg1414.c b/drivers/gpio/gpio-adg1414.c
new file mode 100644
index 000000000..6c0830ee2
--- /dev/null
+++ b/drivers/gpio/gpio-adg1414.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ADG1414 SPST Switch Driver
+ *
+ * Copyright 2024 Analog Devices Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/property.h>
+
+#define ADG1414_MAX_DEVICES 4
+
+struct adg1414_state {
+ struct spi_device *spi;
+ struct gpio_chip chip;
+ struct mutex lock; /* protect sensor state */
+ u32 buf;
+
+ __be32 tx __aligned(ARCH_DMA_MINALIGN);
+};
+
+static void adg1414_set(struct gpio_chip *chip,
+ unsigned int offset,
+ int value)
+{
+ struct adg1414_state *st = gpiochip_get_data(chip);
+ int ret;
+
+ struct spi_transfer xfer = {
+ .tx_buf = &st->tx,
+ .len = st->chip.ngpio / 8,
+ };
+
+ mutex_lock(&st->lock);
+
+ if (value)
+ st->buf |= BIT(offset);
+ else
+ st->buf &= ~BIT(offset);
+
+ st->tx = cpu_to_be32(st->buf << (32 - st->chip.ngpio));
+
+ ret = spi_sync_transfer(st->spi, &xfer, 1);
+ if (ret)
+ goto out;
+
+out:
+ mutex_unlock(&st->lock);
+}
+
+static int adg1414_get(struct gpio_chip *chip,
+ unsigned int offset)
+{
+ struct adg1414_state *st = gpiochip_get_data(chip);
+ int value;
+
+ mutex_lock(&st->lock);
+
+ value = st->buf & BIT(offset);
+
+ mutex_unlock(&st->lock);
+
+ return value;
+}
+
+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 adg1414_state *st;
+ struct gpio_desc *reset;
+ struct device *dev = &spi->dev;
+ u32 num_devices;
+ int ret;
+
+ st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
+ if (!st)
+ return -ENOMEM;
+
+ st->spi = spi;
+
+ /* 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;
+
+ mutex_init(&st->lock);
+
+ return devm_gpiochip_add_data(dev, &st->chip, st);
+}
+
+static const struct of_device_id adg1414_of_match[] = {
+ { .compatible = "adi,adg1414" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, adg1414_of_match);
+
+static struct spi_driver adg1414_driver = {
+ .driver = {
+ .name = "adg1414",
+ .of_match_table = adg1414_of_match,
+ },
+ .probe = adg1414_probe,
+};
+module_spi_driver(adg1414_driver);
+
+MODULE_AUTHOR("Kim Seer Paller <[email protected]>");
+MODULE_DESCRIPTION("ADG1414 SPST Switch Driver");
+MODULE_LICENSE("GPL");
--
2.34.1
On 21/01/2024 11:35, Kim Seer Paller 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 <[email protected]>
> ---
> .../bindings/gpio/gpio-adg1414.yaml | 66 +++++++++++++++++++
> MAINTAINERS | 6 ++
> 2 files changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
> new file mode 100644
> index 000000000..24a51e79f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
Filename like compatible.
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/gpio-adg1414.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADG1414 SPST Switch Driver
"Driver" as Linux driver or some hardware driver?
> +
> +maintainers:
> + - Kim Seer Paller <[email protected]>
> +
> +description:
> + The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS serially-controlled
> + octal SPST switches.
> +
> +properties:
> + compatible:
> + enum:
> + - adi,adg1414
> +
> + reg:
> + maxItems: 1
> +
> + gpio-controller: true
> +
> + '#gpio-cells':
> + const: 2
> +
> + spi-cpha: true
> +
> + reset-gpios:
> + description: GPIO specifier that resets the device.
Drop description, it's obvious. You could instead say something useful,
like name of pin.
> + maxItems: 1
> +
> + '#daisy-chained-devices':
> + description: The number of daisy-chained devices.
> + default: 1
> + minimum: 1
> + maximum: 4
This needs to be added to dtschema or at least its type should be here.
> +
> +required:
> + - compatible
> + - reg
> + - spi-cpha
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof
On 23/01/2024 10:55, Paller, Kim Seer wrote:
> Hi Krzysztof,
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: Monday, January 22, 2024 8:40 PM
>> To: Paller, Kim Seer <[email protected]>; [email protected];
>> [email protected]; [email protected]
>> Cc: Linus Walleij <[email protected]>; Bartosz Golaszewski
>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
>> <[email protected]>; Conor Dooley <[email protected]>
>> Subject: Re: [PATCH 1/2] dt-bindings: gpio: add adg1414
>>
>> [External]
>>
>> On 21/01/2024 11:35, Kim Seer Paller 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 <[email protected]>
>>> ---
>>> .../bindings/gpio/gpio-adg1414.yaml | 66 +++++++++++++++++++
>>> MAINTAINERS | 6 ++
>>> 2 files changed, 72 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
>>> b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
>>> new file mode 100644
>>> index 000000000..24a51e79f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
>>
>> Filename like compatible.
>
> I would like to clarify, is it changing the filename from gpio-adg1414.yaml
> to adg1414.yaml?
No, because you miss vendor prefix. I doubt that it is your compatible...
>
>>> @@ -0,0 +1,66 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id:
>>> +https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/gpio-a
>>> +dg1414.yaml*__;Iw!!A3Ni8CS0y2Y!_Q0zRB8J-
>> inWIIybgOjtThU0toJeUzqaT9Tveg
>>> +XTjvPWiCTh4IAJ5DkNepFdfhXXfxXLnB2wSR9_LgPnvVnFqw2bDuvBI8g$
>>> +$schema:
>>> +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
>>> +aml*__;Iw!!A3Ni8CS0y2Y!_Q0zRB8J-
>> inWIIybgOjtThU0toJeUzqaT9TvegXTjvPWiC
>>> +Th4IAJ5DkNepFdfhXXfxXLnB2wSR9_LgPnvVnFqw2bEIZ1PG4$
>>> +
>>> +title: ADG1414 SPST Switch Driver
>>
>> "Driver" as Linux driver or some hardware driver?
>
> Linux device driver. It seems I overlooked this one, it should be ADG1414 Octal SPST Switches
Then please drop it and double check if your description also has it.
The title and description should describe hardware.
Best regards,
Krzysztof
Hi Krzysztof,
> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Monday, January 22, 2024 8:40 PM
> To: Paller, Kim Seer <[email protected]>; [email protected];
> [email protected]; [email protected]
> Cc: Linus Walleij <[email protected]>; Bartosz Golaszewski
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>
> Subject: Re: [PATCH 1/2] dt-bindings: gpio: add adg1414
>
> [External]
>
> On 21/01/2024 11:35, Kim Seer Paller 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 <[email protected]>
> > ---
> > .../bindings/gpio/gpio-adg1414.yaml | 66 +++++++++++++++++++
> > MAINTAINERS | 6 ++
> > 2 files changed, 72 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
> > b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
> > new file mode 100644
> > index 000000000..24a51e79f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-adg1414.yaml
>
> Filename like compatible.
I would like to clarify, is it changing the filename from gpio-adg1414.yaml
to adg1414.yaml?
> > @@ -0,0 +1,66 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://urldefense.com/v3/__http://devicetree.org/schemas/gpio/gpio-a
> > +dg1414.yaml*__;Iw!!A3Ni8CS0y2Y!_Q0zRB8J-
> inWIIybgOjtThU0toJeUzqaT9Tveg
> > +XTjvPWiCTh4IAJ5DkNepFdfhXXfxXLnB2wSR9_LgPnvVnFqw2bDuvBI8g$
> > +$schema:
> > +https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.y
> > +aml*__;Iw!!A3Ni8CS0y2Y!_Q0zRB8J-
> inWIIybgOjtThU0toJeUzqaT9TvegXTjvPWiC
> > +Th4IAJ5DkNepFdfhXXfxXLnB2wSR9_LgPnvVnFqw2bEIZ1PG4$
> > +
> > +title: ADG1414 SPST Switch Driver
>
> "Driver" as Linux driver or some hardware driver?
Linux device driver. It seems I overlooked this one, it should be ADG1414 Octal SPST Switches
> > +
> > +maintainers:
> > + - Kim Seer Paller <[email protected]>
> > +
Best regards,
Kim
Hi Kim,
On Tue, Jan 23, 2024 at 11:31 AM Paller, Kim Seer
<[email protected]> wrote:
> > Locking here is simple enough that you could use the SPI regmap and
> > get it to do the serialization for you. And then you could possibly
> > reuse the gpio-regmap abstraction and get an even smaller footprint.
>
> I could not seem to figure out how to use the SPI regmap in this case.
> Since the number of daisy-chained devices depends on the length of
> data transferred with continuous transaction, I could not determine
> how to implement that using the SPI regmap. Or maybe I misunderstood
> the statement. However, is it still acceptable to use the current approach?
You just override or wrap with your own read/write callbacks if necessary
by defining a custom static struct regmap_bus.
For example in drivers/gpu/drm/panel/panel-ilitek-ili9322.c
I do this.
It may not save a lot of code in this case but it's still worth it because
we understand what regmap_read/write/update_bits do and reading
and understanding adg1414_set/get cognitively require more from us
as maintainers.
Yours,
Linus Walleij
> -----Original Message-----
> From: Linus Walleij <[email protected]>
> Sent: Wednesday, January 24, 2024 5:51 AM
> To: Paller, Kim Seer <[email protected]>
> Cc: Bartosz Golaszewski <[email protected]>; [email protected];
> [email protected]; [email protected]; Rob Herring
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>
> Subject: Re: [PATCH 2/2] gpio: gpio-adg1414: New driver
>
> [External]
>
> Hi Kim,
>
> On Tue, Jan 23, 2024 at 11:31 AM Paller, Kim Seer
> <[email protected]> wrote:
>
> > > Locking here is simple enough that you could use the SPI regmap and
> > > get it to do the serialization for you. And then you could possibly
> > > reuse the gpio-regmap abstraction and get an even smaller footprint.
> >
> > I could not seem to figure out how to use the SPI regmap in this case.
> > Since the number of daisy-chained devices depends on the length of
> > data transferred with continuous transaction, I could not determine
> > how to implement that using the SPI regmap. Or maybe I misunderstood
> > the statement. However, is it still acceptable to use the current approach?
>
> You just override or wrap with your own read/write callbacks if necessary
> by defining a custom static struct regmap_bus.
>
> For example in drivers/gpu/drm/panel/panel-ilitek-ili9322.c
> I do this.
>
> It may not save a lot of code in this case but it's still worth it because
> we understand what regmap_read/write/update_bits do and reading
> and understanding adg1414_set/get cognitively require more from us
> as maintainers.
Hi Linus,
I'll check this out.
Thanks,
Kim