| 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=Md4pTq5UyDUf44NuByobpYtGGOo0twQnBq9NW+Zt48NdQ@mail.gmail.com>
Date: Thu, 27 Jul 2023 09:55:09 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Okan Sahin <okan.sahin@...log.com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] gpio: ds4520: Add ADI DS4520 GPIO Expander Support
On Mon, Jul 24, 2023 at 8:34 AM Okan Sahin <okan.sahin@...log.com> wrote:
>
> The DS4520 is a 9-bit nonvolatile (NV) I/O expander.
> It offers users a digitally programmable alternative
> to hardware jumpers and mechanical switches that are
> being used to control digital logic node.
>
> Signed-off-by: Okan Sahin <okan.sahin@...log.com>
> ---
This is such a sweet little driver, I love all these abstraction
layers we now have.
Some nits below.
> drivers/gpio/Kconfig | 11 +++++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-ds4520.c | 89 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 101 insertions(+)
> create mode 100644 drivers/gpio/gpio-ds4520.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 13be729710f2..5f89e46d6411 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1000,6 +1000,17 @@ config GPIO_ADNP
> enough to represent all pins, but the driver will assume a
> register layout for 64 pins (8 registers).
>
> +config GPIO_DS4520
> + tristate "DS4520 I2C GPIO expander"
> + select REGMAP_I2C
> + select GPIO_REGMAP
> + help
> + GPIO driver for ADI DS4520 I2C-based GPIO expander.
> + Say yes here to enable the GPIO driver for the ADI DS4520 chip.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called gpio-ds4520.
> +
> config GPIO_GW_PLD
> tristate "Gateworks PLD GPIO Expander"
> depends on OF_GPIO
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index c048ba003367..6f8656d5d617 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_GPIO_DA9052) += gpio-da9052.o
> obj-$(CONFIG_GPIO_DA9055) += gpio-da9055.o
> obj-$(CONFIG_GPIO_DAVINCI) += gpio-davinci.o
> obj-$(CONFIG_GPIO_DLN2) += gpio-dln2.o
> +obj-$(CONFIG_GPIO_DS4520) += gpio-ds4520.o
> obj-$(CONFIG_GPIO_DWAPB) += gpio-dwapb.o
> obj-$(CONFIG_GPIO_EIC_SPRD) += gpio-eic-sprd.o
> obj-$(CONFIG_GPIO_EM) += gpio-em.o
> diff --git a/drivers/gpio/gpio-ds4520.c b/drivers/gpio/gpio-ds4520.c
> new file mode 100644
> index 000000000000..577ffb4b8c50
> --- /dev/null
> +++ b/drivers/gpio/gpio-ds4520.c
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 Analog Devices, Inc.
> + * Driver for the DS4520 I/O Expander
> + */
> +
> +#include <linux/device.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/regmap.h>
> +#include <linux/i2c.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +
> +#define DS4520_PULLUP0 0xF0
> +#define DS4520_IO_CONTROL0 0xF2
> +#define DS4520_IO_STATUS0 0xF8
> +
> +static const struct regmap_config ds4520_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> +static int ds4520_gpio_probe(struct i2c_client *client)
> +{
> + struct gpio_regmap_config config = { };
> + struct device *dev = &client->dev;
> + struct regmap *regmap;
> + u32 ngpio;
> + u32 base;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "reg", &base);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "Missing 'reg' property.\n");
> + return -EINVAL;
> + }
Please add a newline here.
> + ret = device_property_read_u32(dev, "ngpios", &ngpio);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "Missing 'ngpios' property.\n");
> + return -EINVAL;
> + }
And here.
> + regmap = devm_regmap_init_i2c(client, &ds4520_regmap_config);
> + if (IS_ERR(regmap)) {
> + ret = PTR_ERR(regmap);
> + dev_err_probe(dev, ret,
> + "Failed to allocate register map\n");
> + return ret;
> + }
> +
> + config.regmap = regmap;
> + config.parent = dev;
> + config.ngpio = ngpio;
> +
> + config.reg_dat_base = base + DS4520_IO_STATUS0;
> + config.reg_set_base = base + DS4520_PULLUP0;
> + config.reg_dir_out_base = base + DS4520_IO_CONTROL0;
> +
> + return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &config));
> +}
> +
> +static const struct of_device_id ds4520_gpio_of_match_table[] = {
> + {
> + .compatible = "adi,ds4520-gpio"
Keep this on a single line.
> + },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ds4520_gpio_of_match_table);
> +
> +static const struct i2c_device_id ds4520_gpio_id_table[] = {
> + { "ds4520-gpio" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, ds4520_gpio_id_table);
> +
> +static struct i2c_driver ds4520_gpio_driver = {
> + .driver = {
> + .name = "ds4520-gpio",
> + .of_match_table = ds4520_gpio_of_match_table,
> + },
> + .probe_new = ds4520_gpio_probe,
> + .id_table = ds4520_gpio_id_table,
> +};
> +module_i2c_driver(ds4520_gpio_driver);
> +
> +MODULE_DESCRIPTION("DS4520 I/O Expander");
> +MODULE_AUTHOR("Okan Sahin <okan.sahin@...log.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>
Bart
Powered by blists - more mailing lists