diff mbox series

[v2,2/2] gpio: fxl6408: add I2C GPIO expander driver

Message ID 20230313113308.157930-3-francesco@dolcini.it
State New
Headers show
Series gpio: fxl6408: add I2C GPIO expander driver | expand

Commit Message

Francesco Dolcini March 13, 2023, 11:33 a.m. UTC
From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>

Add minimal driver for Fairchild FXL6408 8-bit I2C-controlled GPIO expander
using the generic regmap based GPIO driver (GPIO_REGMAP).

The driver implements setting the GPIO direction, reading inputs
and writing outputs.

In addition to that the FXL6408 has the following functionalities:
- allows to monitor input ports for data transitions with an interrupt pin
- all inputs can be configured with pull-up or pull-down resistors

Datasheet: https://www.onsemi.com/download/data-sheet/pdf/fxl6408-d.pdf
Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
v2:
 * remove includes: <linux/gpio.h> and <linux/of_platform.h>
 * add missing and required select REGMAP_I2C in KConfig
 * use dev_err_probe()
 * add "Datasheet:" tag in commit message
 * improve KConfig help section
 * fix newlines, multi-line comments and trailing commas
---
 drivers/gpio/Kconfig        |  10 +++
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-fxl6408.c | 152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 drivers/gpio/gpio-fxl6408.c

Comments

Francesco Dolcini March 13, 2023, 1:09 p.m. UTC | #1
On Mon, Mar 13, 2023 at 02:24:47PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 13, 2023 at 1:33 PM Francesco Dolcini <francesco@dolcini.it> wrote:
> > +config GPIO_FXL6408
> > +       tristate "FXL6408 I2C GPIO expander"
> > +       select GPIO_REGMAP
> 
> > +       select REGMAP_I2C
> 
> Somebody pointed out that this might require
> 
>     depends on I2C
> 
> being added as well.

This is already there, I would prefer to not duplicate it neither do it
differently from all the other I2C GPIO drivers.

menu "I2C GPIO expanders"
	depends on I2C

> > +#define FXL6408_MAX_REGISTER           0x13
> 
> This is used as a range, but why? If we can have a proper name for
> this register, why bother dumping all this or even having access to?

So, I see 2 options:
1. Use FXL6408_REG_INPUT_STATUS instead of FXL6408_MAX_REGISTER
2. #define FXL6408_REG_INT_STS 0x13 and use it instead

I think that there are trivial benefits on both, with option 2 being the
correct hardware description and that would not need to be changed when adding
additional functionalities to the driver.

Which solution do we prefer for v3?
My preference would be to define FXL6408_REG_INT_STS and use it instead.

Francesco
diff mbox series

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 13be729710f2..56a73007ebcb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1000,6 +1000,16 @@  config GPIO_ADNP
 	  enough to represent all pins, but the driver will assume a
 	  register layout for 64 pins (8 registers).
 
+config GPIO_FXL6408
+	tristate "FXL6408 I2C GPIO expander"
+	select GPIO_REGMAP
+	select REGMAP_I2C
+	help
+	  GPIO driver for Fairchild Semiconductor FXL6408 GPIO expander.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called gpio-fxl6408.
+
 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..12027f4c3bee 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -60,6 +60,7 @@  obj-$(CONFIG_GPIO_EP93XX)		+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_EXAR)			+= gpio-exar.o
 obj-$(CONFIG_GPIO_F7188X)		+= gpio-f7188x.o
 obj-$(CONFIG_GPIO_FTGPIO010)		+= gpio-ftgpio010.o
+obj-$(CONFIG_GPIO_FXL6408)		+= gpio-fxl6408.o
 obj-$(CONFIG_GPIO_GE_FPGA)		+= gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM)		+= gpio-gpio-mm.o
 obj-$(CONFIG_GPIO_GRGPIO)		+= gpio-grgpio.o
diff --git a/drivers/gpio/gpio-fxl6408.c b/drivers/gpio/gpio-fxl6408.c
new file mode 100644
index 000000000000..2560fe99c2d6
--- /dev/null
+++ b/drivers/gpio/gpio-fxl6408.c
@@ -0,0 +1,152 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * FXL6408 GPIO driver
+ *
+ * Copyright 2023 Toradex
+ *
+ * Author: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
+ */
+
+#include <linux/gpio/regmap.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define FXL6408_REG_DEVICE_ID		0x01
+#define FXL6408_MF_FAIRCHILD		0b101
+#define FXL6408_MF_SHIFT		5
+
+/* Bits set here indicate that the GPIO is an output. */
+#define FXL6408_REG_IO_DIR		0x03
+
+/*
+ * Bits set here, when the corresponding bit of IO_DIR is set, drive
+ * the output high instead of low.
+ */
+#define FXL6408_REG_OUTPUT		0x05
+
+/* Bits here make the output High-Z, instead of the OUTPUT value. */
+#define FXL6408_REG_OUTPUT_HIGH_Z	0x07
+
+/* Returns the current status (1 = HIGH) of the input pins. */
+#define FXL6408_REG_INPUT_STATUS	0x0f
+
+#define FXL6408_MAX_REGISTER		0x13
+
+#define FXL6408_NGPIO			8
+
+static const struct regmap_range rd_range[] = {
+	{ FXL6408_REG_DEVICE_ID, FXL6408_REG_DEVICE_ID },
+	{ FXL6408_REG_IO_DIR, FXL6408_REG_OUTPUT },
+	{ FXL6408_REG_INPUT_STATUS, FXL6408_REG_INPUT_STATUS },
+};
+
+static const struct regmap_range wr_range[] = {
+	{ FXL6408_REG_DEVICE_ID, FXL6408_REG_DEVICE_ID },
+	{ FXL6408_REG_IO_DIR, FXL6408_REG_OUTPUT },
+	{ FXL6408_REG_OUTPUT_HIGH_Z, FXL6408_REG_OUTPUT_HIGH_Z },
+};
+
+static const struct regmap_range volatile_range[] = {
+	{ FXL6408_REG_DEVICE_ID, FXL6408_REG_DEVICE_ID },
+	{ FXL6408_REG_INPUT_STATUS, FXL6408_REG_INPUT_STATUS },
+};
+
+static const struct regmap_access_table rd_table = {
+	.yes_ranges = rd_range,
+	.n_yes_ranges = ARRAY_SIZE(rd_range),
+};
+
+static const struct regmap_access_table wr_table = {
+	.yes_ranges = wr_range,
+	.n_yes_ranges = ARRAY_SIZE(wr_range),
+};
+
+static const struct regmap_access_table volatile_table = {
+	.yes_ranges = volatile_range,
+	.n_yes_ranges = ARRAY_SIZE(volatile_range),
+};
+
+static const struct regmap_config regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+
+	.max_register = FXL6408_MAX_REGISTER,
+	.wr_table = &wr_table,
+	.rd_table = &rd_table,
+	.volatile_table = &volatile_table,
+
+	.cache_type = REGCACHE_RBTREE,
+	.num_reg_defaults_raw = FXL6408_MAX_REGISTER + 1,
+};
+
+static int fxl6408_identify(struct device *dev, struct regmap *regmap)
+{
+	int val, ret;
+
+	ret = regmap_read(regmap, FXL6408_REG_DEVICE_ID, &val);
+	if (ret) {
+		dev_err(dev, "error %d reading DEVICE_ID\n", ret);
+	} else if (val >> FXL6408_MF_SHIFT != FXL6408_MF_FAIRCHILD) {
+		dev_err(dev, "invalid device id 0x%02x\n", val);
+		ret = -ENODEV;
+	}
+
+	return ret;
+}
+
+static int fxl6408_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	int ret;
+	struct gpio_regmap_config gpio_config = {
+		.parent = dev,
+		.ngpio = FXL6408_NGPIO,
+		.reg_dat_base = GPIO_REGMAP_ADDR(FXL6408_REG_INPUT_STATUS),
+		.reg_set_base = GPIO_REGMAP_ADDR(FXL6408_REG_OUTPUT),
+		.reg_dir_out_base = GPIO_REGMAP_ADDR(FXL6408_REG_IO_DIR),
+		.ngpio_per_reg = FXL6408_NGPIO,
+	};
+
+	gpio_config.regmap = devm_regmap_init_i2c(client, &regmap);
+	if (IS_ERR(gpio_config.regmap))
+		return dev_err_probe(dev, PTR_ERR(gpio_config.regmap),
+				     "failed to allocate register map\n");
+
+	ret = fxl6408_identify(dev, gpio_config.regmap);
+	if (ret)
+		return ret;
+
+	/* Disable High-Z of outputs, so that our OUTPUT updates actually take effect. */
+	ret = regmap_write(gpio_config.regmap, FXL6408_REG_OUTPUT_HIGH_Z, 0);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to write 'output high Z' register\n");
+
+	return PTR_ERR_OR_ZERO(devm_gpio_regmap_register(dev, &gpio_config));
+}
+
+static const __maybe_unused struct of_device_id fxl6408_dt_ids[] = {
+	{ .compatible = "fcs,fxl6408" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, fxl6408_dt_ids);
+
+static const struct i2c_device_id fxl6408_id[] = {
+	{ "fxl6408", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, fxl6408_id);
+
+static struct i2c_driver fxl6408_driver = {
+	.driver = {
+		.name	= "fxl6408",
+		.of_match_table = fxl6408_dt_ids,
+	},
+	.probe_new	= fxl6408_probe,
+	.id_table	= fxl6408_id,
+};
+module_i2c_driver(fxl6408_driver);
+
+MODULE_AUTHOR("Emanuele Ghidoli <emanuele.ghidoli@toradex.com>");
+MODULE_DESCRIPTION("FXL6408 GPIO driver");
+MODULE_LICENSE("GPL");