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....@thorcom.co.uk>");
+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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to