| 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: <CAMRc=MeGVAO8Fr4U5ai-OgEmX5gXeddLDKyRC+FQia1TH64m+Q@mail.gmail.com>
Date: Mon, 22 Jan 2024 14:00:07 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Kim Seer Paller <kimseer.paller@...log.com>
Cc: linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Linus Walleij <linus.walleij@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>
Subject: Re: [PATCH 2/2] gpio: gpio-adg1414: New driver
On Sun, Jan 21, 2024 at 11:35 AM Kim Seer Paller
<kimseer.paller@...log.com> wrote:
>
> The ADG1414 is a 9.5 Ω RON ±15 V/+12 V/±5 V iCMOS Serially-Controlled
> Octal SPST Switches
Switch? (singular)
>
> Signed-off-by: Kim Seer Paller <kimseer.paller@...log.com>
> ---
> 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 <kimseer.paller@...log.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@...e.com>
> 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>
Can you keep the headers in alphabetical order please?
> +#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 */
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.
> + u32 buf;
> +
> + __be32 tx __aligned(ARCH_DMA_MINALIGN);
> +};
Please drop the tabs from the struct, I personally don't think they
add to readability.
> +
> +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 <kimseer.paller@...log.com>");
> +MODULE_DESCRIPTION("ADG1414 SPST Switch Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>
Bart
Powered by blists - more mailing lists