| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a1577c7ce81d039f47e189e130295b76447b05c2.camel@gmail.com>
Date: Mon, 03 Nov 2025 11:18:42 +0000
From: Nuno Sá <noname.nuno@...il.com>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>, Linus Walleij
<linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>, Rob
Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
Dooley <conor+dt@...nel.org>, linux-gpio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] gpio: adg1712: add driver support
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@...log.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@...log.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á
>
Powered by blists - more mailing lists