[v4,2/2] gpio: ds4520: Add ADI DS4520 GPIO Expander Support
Commit Message
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@analog.com>
---
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
Comments
On 27/07/2023 10:52, Okan Sahin 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@analog.com>
...
> +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;
Nope.
> + }
> +
> + ret = device_property_read_u32(dev, "ngpios", &ngpio);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "Missing 'ngpios' property.\n");
> + return -EINVAL;
Nope.
> + }
> +
> + 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;
That's not correct syntax. What did you receive in previous
comments/feedback?
Best regards,
Krzysztof
On 27/07/2023 11:03, Krzysztof Kozlowski wrote:
> On 27/07/2023 10:52, Okan Sahin 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@analog.com>
>
> ...
>
>> +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;
>
> Nope.
>
>> + }
>> +
>> + ret = device_property_read_u32(dev, "ngpios", &ngpio);
>> + if (ret) {
>> + dev_err_probe(dev, ret,
>> + "Missing 'ngpios' property.\n");
>> + return -EINVAL;
>
> Nope.
>
>> + }
>> +
>> + 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;
>
> That's not correct syntax. What did you receive in previous
> comments/feedback?
Hm, I might be mixing patches, so maybe you never received feedback on
this. Anyway, all these three lines must be one line:
return dev_err_probe()
Previous places should be fixed similar way.
Best regards,
Krzysztof
On Thu, Jul 27, 2023 at 10:53 AM Okan Sahin <okan.sahin@analog.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@analog.com>
> ---
> 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..0a9fdbfed6ee
> --- /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;
> + }
> +
> + ret = device_property_read_u32(dev, "ngpios", &ngpio);
> + if (ret) {
> + dev_err_probe(dev, ret,
> + "Missing 'ngpios' property.\n");
> + return -EINVAL;
> + }
> +
> + 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" },
> + { }
> +};
> +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,
I missed this previously but i2c drivers have completed the conversion
to the new probe syntax, so please use regular probe.
And please address Krzysztof's comments as well.
Thanks,
Bart
> + .id_table = ds4520_gpio_id_table,
> +};
> +module_i2c_driver(ds4520_gpio_driver);
> +
> +MODULE_DESCRIPTION("DS4520 I/O Expander");
> +MODULE_AUTHOR("Okan Sahin <okan.sahin@analog.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>
@@ -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
@@ -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
new file mode 100644
@@ -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;
+ }
+
+ ret = device_property_read_u32(dev, "ngpios", &ngpio);
+ if (ret) {
+ dev_err_probe(dev, ret,
+ "Missing 'ngpios' property.\n");
+ return -EINVAL;
+ }
+
+ 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" },
+ { }
+};
+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@analog.com>");
+MODULE_LICENSE("GPL");