[PATCH] added pca9570 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



First, apologies, this is my first Linux kernel patch in at least 15 years. While I have spent a few hours reading various pieces of documentation about the Linux kernel development processes, I have probably missed some critical files entirely.

Also, there are probably a few coding issues in this patch.

This driver has only been tested to work on our custom ARM board, where we give it the following device tree info:

arch/arm/boot/dts/imx6qdl-smarcfimx6.dtsi:

...
&i2c1 {
        ...
        pca9570: pca9570@24 {
                compatible = "pca9570";
                reg = <0x24>;
                gpio-controller;
        };
};

I have some questions:

- Is the "value", with which struct gpio_chip.set is called, guaranteed to be 0 or 1?

- Do I need to implement any form of locking around this driver, or is that done for me in layers above?

- The pca9571 is an 8-bit version of this chip. Is leaving pca9571 support to a later patch the best idea? (I do not have one to test, nor any way to test one, at the moment.)

- This code was written and tested against a vendor-modified 4.1.15 kernel tree. This patch is made against the stable 4.9.9. To make it compile against the newer kernel I had to replace references to struct gpio_chip.dev with struct gpio_chip.parent, as that struct member had been renamed between kernel versions.

- Should this driver be in the staging directory?

- If so, how to do this, as staging does not seem to have either a staging/gpio nor a staging/include/linux/i2c directory to put the driver's files in.

Please let me know what else I need to read and do, to make this patch into something that stands a chance of being accepted upstream.

All the best,

Chris.


---
 drivers/gpio/Kconfig        |   7 ++
 drivers/gpio/Makefile       |   1 +
drivers/gpio/gpio-pca9570.c | 253 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c/pca9570.h |  22 ++++
 4 files changed, 283 insertions(+)
 create mode 100644 drivers/gpio/gpio-pca9570.c
 create mode 100644 include/linux/i2c/pca9570.h

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0504307..a068cdb 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -762,6 +762,13 @@ config GPIO_PCA953X

 	  40 bits:	pca9505, pca9698

+config GPIO_PCA9570
+	tristate "PCA9570 GPO expander"
+	depends on I2C
+	help
+	  Say yes here to provide access to the PCA9570 SMBus GPO expander,
+	  made by NXP.
+
 config GPIO_PCA953X_IRQ
 	bool "Interrupt controller support for PCA953x"
 	depends on GPIO_PCA953X=y
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index becb96c..ec7946f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -90,6 +90,7 @@ obj-$(CONFIG_GPIO_MXS)		+= gpio-mxs.o
 obj-$(CONFIG_GPIO_OCTEON)	+= gpio-octeon.o
 obj-$(CONFIG_GPIO_OMAP)		+= gpio-omap.o
 obj-$(CONFIG_GPIO_PCA953X)	+= gpio-pca953x.o
+obj-$(CONFIG_GPIO_PCA9570)	+= gpio-pca9570.o
 obj-$(CONFIG_GPIO_PCF857X)	+= gpio-pcf857x.o
 obj-$(CONFIG_GPIO_PCH)		+= gpio-pch.o
 obj-$(CONFIG_GPIO_PCI_IDIO_16)	+= gpio-pci-idio-16.o
diff --git a/drivers/gpio/gpio-pca9570.c b/drivers/gpio/gpio-pca9570.c
new file mode 100644
index 0000000..b3dc0e3
--- /dev/null
+++ b/drivers/gpio/gpio-pca9570.c
@@ -0,0 +1,253 @@
+/*
+ * Driver for pca9570 I2C GPO expanders
+ * Note: 9570 is at 0x24 - these value must be set
+ * in device tree.
+ *
+ * Copyright (C) 2017 Chris Dew, Thorcom Systems Ltd.
+ *
+ * Derived from drivers/gpio/gpio-max732x.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/i2c/pca9570.h>
+#include <linux/of.h>
+
+#define PCA9570_GPIO_COUNT 4
+
+enum {
+	PCA9570,
+};
+
+static const struct i2c_device_id pca9570_id[] = {
+	{ "pca9570", PCA9570 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, pca9570_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id pca9570_of_table[] = {
+ { .compatible = "nxp,pca9570" }, /* FIXME: I just put "nxp" in here as other drivers had a manufacturer name */
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pca9570_of_table);
+#endif
+
+
+struct pca9570_chip {
+	struct gpio_chip gpio_chip;
+	struct i2c_client *client;
+	struct mutex lock; /* FIXME: does this driver need to do any locking? */
+	uint8_t	reg;
+};
+
+static inline struct pca9570_chip *to_pca9570(struct gpio_chip *gc)
+{
+	return container_of(gc, struct pca9570_chip, gpio_chip);
+}
+
+static struct pca9570_platform_data *of_gpio_pca9570(struct device *dev)
+{
+	struct pca9570_platform_data *pdata;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->gpio_base = -1;
+
+	return pdata;
+}
+
+static int pca9570_readb(struct pca9570_chip *chip, uint8_t *val)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte(chip->client);
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "failed reading\n");
+		return ret;
+	}
+
+	*val = (uint8_t)ret;
+	return 0;
+}
+
+static int pca9570_writeb(struct pca9570_chip *chip, uint8_t val)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte(chip->client, val);
+	if (ret < 0) {
+		dev_err(&chip->client->dev, "failed writing\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int pca9570_gpio_get_value(struct gpio_chip *gc, unsigned off)
+{
+	struct pca9570_chip *chip = to_pca9570(gc);
+
+	uint8_t mask = (uint8_t) 1 << off;
+	return chip->reg & mask;
+}
+
+static void pca9570_gpio_set_value(struct gpio_chip *gc, unsigned off,
+					int val)
+{
+	struct pca9570_chip *chip = to_pca9570(gc);
+
+	uint8_t mask = (uint8_t) 1 << off;
+
+	chip->reg &= ~mask;
+	chip->reg |= mask * val;
+
+	pca9570_writeb(chip, chip->reg);
+}
+
+static int pca9570_setup_gpio(struct pca9570_chip *chip,
+					const struct i2c_device_id *id,
+					unsigned gpio_start)
+{
+	struct gpio_chip *gc = &chip->gpio_chip;
+
+	gc->set = pca9570_gpio_set_value;
+	gc->get = pca9570_gpio_get_value;
+
+	gc->can_sleep = true;
+
+	gc->base = gpio_start;
+	gc->ngpio = PCA9570_GPIO_COUNT;
+	gc->label = chip->client->name;
+	gc->owner = THIS_MODULE;
+
+	gc->parent = &chip->client->dev;
+
+	return PCA9570_GPIO_COUNT;
+}
+
+static int pca9570_probe(struct i2c_client *client,
+				   const struct i2c_device_id *id)
+{
+	struct pca9570_platform_data *pdata;
+	struct device_node *node;
+	struct pca9570_chip *chip;
+	int ret = 0, nr_port;
+
+	pdata = dev_get_platdata(&client->dev);
+	node = client->dev.of_node;
+
+	if (!pdata && node)
+		pdata = of_gpio_pca9570(&client->dev);
+
+	if (!pdata) {
+		dev_dbg(&client->dev, "no platform data\n");
+		return -EINVAL;
+	}
+
+	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
+	if (chip == NULL)
+		return -ENOMEM;
+	chip->client = client;
+
+	nr_port = pca9570_setup_gpio(chip, id, pdata->gpio_base);
+	chip->gpio_chip.parent = &client->dev;
+
+	ret = gpiochip_add(&chip->gpio_chip);
+	if (ret)
+		goto out_failed;
+
+	if (pdata && pdata->setup) {
+		ret = pdata->setup(client, chip->gpio_chip.base,
+				chip->gpio_chip.ngpio, pdata->context);
+		if (ret < 0)
+			dev_warn(&client->dev, "setup failed, %d\n", ret);
+	}
+
+	i2c_set_clientdata(client, chip);
+
+	ret = pca9570_readb(chip, &chip->reg);
+	if (ret)
+		goto out_failed;
+
+	return 0;
+
+out_failed:
+	if (chip->client)
+		i2c_unregister_device(chip->client);
+
+	return -1;
+}
+
+static int pca9570_remove(struct i2c_client *client)
+{
+	struct pca9570_platform_data *pdata = dev_get_platdata(&client->dev);
+	struct pca9570_chip *chip = i2c_get_clientdata(client);
+
+	if (pdata && pdata->teardown) {
+		int ret;
+
+		ret = pdata->teardown(client, chip->gpio_chip.base,
+				chip->gpio_chip.ngpio, pdata->context);
+		if (ret < 0) {
+			dev_err(&client->dev, "%s failed, %d\n",
+					"teardown", ret);
+			return ret;
+		}
+	}
+
+	gpiochip_remove(&chip->gpio_chip);
+
+	return 0;
+}
+
+static struct i2c_driver pca9570_driver = {
+	.driver = {
+		.name		= "pca9570",
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(pca9570_of_table),
+	},
+	.probe		= pca9570_probe,
+	.remove		= pca9570_remove,
+	.id_table	= pca9570_id,
+};
+
+static int __init pca9570_init(void)
+{
+	return i2c_add_driver(&pca9570_driver);
+}
+/* register after i2c postcore initcall and before
+ * subsys initcalls that may rely on these GPIOs
+ */
+subsys_initcall(pca9570_init);
+
+static void __exit pca9570_exit(void)
+{
+	i2c_del_driver(&pca9570_driver);
+}
+module_exit(pca9570_exit);
+
+MODULE_AUTHOR("Chris Dew <chris.dew@xxxxxxxxxxxxx>");
+MODULE_DESCRIPTION("pca9570");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c/pca9570.h b/include/linux/i2c/pca9570.h
new file mode 100644
index 0000000..77207ed
--- /dev/null
+++ b/include/linux/i2c/pca9570.h
@@ -0,0 +1,22 @@
+#ifndef __LINUX_I2C_PCA9570_H
+#define __LINUX_I2C_PCA9570_H
+
+/* platform data for the PCA9570 4 GPO expander driver */
+
+struct pca9570_platform_data {
+	/* number of the first GPIO */
+	unsigned	gpio_base;
+
+	/* interrupt base */
+	int		irq_base;
+
+	void		*context;	/* param to setup/teardown */
+
+	int		(*setup)(struct i2c_client *client,
+				unsigned gpio, unsigned ngpio,
+				void *context);
+	int		(*teardown)(struct i2c_client *client,
+				unsigned gpio, unsigned ngpio,
+				void *context);
+};
+#endif /* __LINUX_I2C_PCA9570_H */
--
2.7.4
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux