[PATCH v2 0/2] gpio: Add driver for SPI serializers

[Andrew F. Davis]

This series adds a GPI(General Purpose Input) driver for generic
parallel-in/serial-out shift registers, such as the SN65HVS882
that this driver was tested on. This should also work for the rest
of the SN65HVS88x series as well as other 74x165 style devices.

Changes from v1[0]:
- Use new gpiochip_add_data
- Rebased on v4.5-rc1
- Small fixes

[0] http://www.spinics.net/lists/linux-gpio/msg10563.html

Andrew F. Davis (2):
dt-bindings: GPIO: Add generic serializer binding
gpio: Add driver for SPI serializers

.../devicetree/bindings/gpio/gpio-pisosr.txt | 34 ++++
drivers/gpio/Kconfig | 6 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-pisosr.c | 188 +++++++++++++++++++++
4 files changed, 229 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pisosr.txt
create mode 100644 drivers/gpio/gpio-pisosr.c

--
2.7.0

[Andrew F. Davis]

Add binding for generic parallel-in/serial-out shift register devices
used as GPIO.

Signed-off-by: Andrew F. Davis <a...@ti.com>
---
.../devicetree/bindings/gpio/gpio-pisosr.txt | 34 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pisosr.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-pisosr.txt b/Documentation/devicetree/bindings/gpio/gpio-pisosr.txt
new file mode 100644
index 0000000..e69e8ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-pisosr.txt
@@ -0,0 +1,34 @@
+Generic Parallel-in/Serial-out Shift Register GPIO Driver
+
+This binding describes generic parallel-in/serial-out shift register
+devices that can be used for GPI (General Purpose Input). This includes
+SN74165 serial-out shift registers and the SN65HVS88x series of
+industrial serializers.
+
+Required properties:
+ - compatible : Should be "pisosr-gpio".
+ - gpio-controller : Marks the device node as a GPIO controller.
+ - #gpio-cells : Should be two. For consumer use see gpio.txt.
+
+Optional properties:
+ - ngpios : Number of GPIO lines, default is 8.
+ - load-gpios : GPIO pin specifier attached to load enable, this
+ pin is pulsed before reading from the device to
+ load input pin values into the the device.
+
+For other required and optional properties of SPI slave
+nodes please refer to ../spi/spi-bus.txt.
+
+Example:
+
+ sn65hvs882@0 {
+ compatible = "pisosr-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ load-gpios = <&gpio2 23 GPIO_ACTIVE_LOW>;
+
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cpol;
+ };
--
2.7.0

[Andrew F. Davis]

Add generic parallel-in/serial-out shift register GPIO driver.

This includes SPI compatible devices like SN74165 serial-out shift
registers and the SN65HVS88x series of industrial serializers that can
be read over the SPI bus and used for GPI (General Purpose Input).

Signed-off-by: Andrew F. Davis <a...@ti.com>
---

drivers/gpio/Kconfig | 6 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-pisosr.c | 188 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 195 insertions(+)
create mode 100644 drivers/gpio/gpio-pisosr.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c88dd24..bec3489 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1011,6 +1011,12 @@ config GPIO_MC33880
SPI driver for Freescale MC33880 high-side/low-side switch.
This provides GPIO interface supporting inputs and outputs.

+config GPIO_PISOSR
+ tristate "Generic parallel-in/serial-out shift register"
+ help
+ GPIO driver for SPI compatible parallel-in/serial-out shift
+ registers. These are input only devices.
+
endmenu

menu "SPI or I2C GPIO expanders"
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ece7d7c..8e4f09f 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_GPIO_OMAP) += gpio-omap.o
obj-$(CONFIG_GPIO_PCA953X) += gpio-pca953x.o
obj-$(CONFIG_GPIO_PCF857X) += gpio-pcf857x.o
obj-$(CONFIG_GPIO_PCH) += gpio-pch.o
+obj-$(CONFIG_GPIO_PISOSR) += gpio-pisosr.o
obj-$(CONFIG_GPIO_PL061) += gpio-pl061.o
obj-$(CONFIG_GPIO_PXA) += gpio-pxa.o
obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c
new file mode 100644
index 0000000..58ea08d
--- /dev/null
+++ b/drivers/gpio/gpio-pisosr.c
@@ -0,0 +1,188 @@
+/*
+ * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com/
+ * Andrew F. Davis <a...@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi.h>
+
+#define DEFAULT_NGPIO 8
+
+/**
+ * struct pisosr_gpio - GPIO driver data
+ * @chip: GPIO controller chip
+ * @spi: SPI device pointer
+ * @buffer: Buffer for device reads
+ * @buffer_size: Size of buffer
+ * @load_gpio: GPIO pin used to load input into device
+ * @lock: Protects read sequences
+ */
+struct pisosr_gpio {
+ struct gpio_chip chip;
+ struct spi_device *spi;
+ u8 *buffer;
+ size_t buffer_size;
+ struct gpio_desc *load_gpio;
+ struct mutex lock;
+};
+
+static int pisosr_gpio_refresh(struct pisosr_gpio *gpio)
+{
+ int ret;
+
+ mutex_lock(&gpio->lock);
+
+ if (gpio->load_gpio) {
+ gpiod_set_value(gpio->load_gpio, 1);
+ udelay(1); /* registers load time (~10ns) */
+ gpiod_set_value(gpio->load_gpio, 0);
+ udelay(1); /* registers recovery time (~5ns) */
+ }
+
+ ret = spi_read(gpio->spi, gpio->buffer, gpio->buffer_size);
+ if (ret)
+ return ret;
+
+ mutex_unlock(&gpio->lock);
+
+ return 0;
+}
+
+static int pisosr_gpio_get_direction(struct gpio_chip *chip,
+ unsigned offset)
+{
+ /* This device always input */
+ return 1;
+}
+
+static int pisosr_gpio_direction_input(struct gpio_chip *chip,
+ unsigned offset)
+{
+ /* This device always input */
+ return 0;
+}
+
+static int pisosr_gpio_direction_output(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ /* This device is input only */
+ return -EINVAL;
+}
+
+static int pisosr_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct pisosr_gpio *gpio = gpiochip_get_data(chip);
+
+ /* Refresh may not always be needed */
+ pisosr_gpio_refresh(gpio);
+
+ return (gpio->buffer[offset / 8] >> (offset % 8)) & 0x1;
+}
+
+static struct gpio_chip template_chip = {
+ .label = "pisosr-gpio",
+ .owner = THIS_MODULE,
+ .get_direction = pisosr_gpio_get_direction,
+ .direction_input = pisosr_gpio_direction_input,
+ .direction_output = pisosr_gpio_direction_output,
+ .get = pisosr_gpio_get,
+ .base = -1,
+ .ngpio = DEFAULT_NGPIO,
+ .can_sleep = true,
+};
+
+static int pisosr_gpio_probe(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct pisosr_gpio *gpio;
+ int ret;
+
+ gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
+ if (!gpio)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, gpio);
+
+ gpio->chip = template_chip;
+ gpio->chip.parent = dev;
+ of_property_read_u16(dev->of_node, "ngpios", &gpio->chip.ngpio);
+
+ gpio->spi = spi;
+
+ gpio->buffer_size = DIV_ROUND_UP(gpio->chip.ngpio, 8);
+ gpio->buffer = devm_kzalloc(dev, gpio->buffer_size, GFP_KERNEL);
+ if (!gpio->buffer)
+ return -ENOMEM;
+
+ gpio->load_gpio = devm_gpiod_get(dev, "load", GPIOD_OUT_LOW);
+ if (IS_ERR(gpio->load_gpio)) {
+ ret = PTR_ERR(gpio->load_gpio);
+ if (ret != -ENOENT && ret != -ENOSYS) {
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "Unable to allocate load GPIO\n");
+ return ret;
+ }
+ gpio->load_gpio = NULL;
+ }
+
+ mutex_init(&gpio->lock);
+
+ ret = gpiochip_add_data(&gpio->chip, gpio);
+ if (ret < 0) {
+ dev_err(dev, "Unable to register gpiochip\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int pisosr_gpio_remove(struct spi_device *spi)
+{
+ struct pisosr_gpio *gpio = spi_get_drvdata(spi);
+
+ gpiochip_remove(&gpio->chip);
+
+ mutex_destroy(&gpio->lock);
+
+ return 0;
+}
+
+static const struct spi_device_id pisosr_gpio_id_table[] = {
+ { "pisosr-gpio", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, pisosr_gpio_id_table);
+
+static const struct of_device_id pisosr_gpio_of_match_table[] = {
+ { .compatible = "pisosr-gpio", },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, pisosr_gpio_of_match_table);
+
+static struct spi_driver pisosr_gpio_driver = {
+ .driver = {
+ .name = "pisosr-gpio",
+ .of_match_table = pisosr_gpio_of_match_table,
+ },
+ .probe = pisosr_gpio_probe,
+ .remove = pisosr_gpio_remove,
+ .id_table = pisosr_gpio_id_table,
+};
+module_spi_driver(pisosr_gpio_driver);
+
+MODULE_AUTHOR("Andrew F. Davis <a...@ti.com>");
+MODULE_DESCRIPTION("SPI Compatible PISO Shift Register GPIO Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.0

[Rob Herring]

On Mon, Jan 25, 2016 at 10:37:30AM -0600, Andrew F. Davis wrote:
> Add binding for generic parallel-in/serial-out shift register devices
> used as GPIO.
>
> Signed-off-by: Andrew F. Davis <a...@ti.com>
> ---
> .../devicetree/bindings/gpio/gpio-pisosr.txt | 34 ++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-pisosr.txt

One nit, otherwise:

Acked-by: Rob Herring <ro...@kernel.org>

should be gpio@0

[Linus Walleij]

On Mon, Jan 25, 2016 at 5:37 PM, Andrew F. Davis <a...@ti.com> wrote:

> Add binding for generic parallel-in/serial-out shift register devices
> used as GPIO.
>
> Signed-off-by: Andrew F. Davis <a...@ti.com>

(...)

> +Required properties:
> + - compatible : Should be "pisosr-gpio".

As mentioned I'd like some vendor examples here with dual strings.
"ti,foo", "pisosr-gpio"; is fine.

> + sn65hvs882@0 {

Fix Rob's comment.

> + compatible = "pisosr-gpio";

And a dual-string in the example.

Yours,
Linus Walleij

[Rob Herring]

On Thu, Jan 28, 2016 at 4:59 AM, Linus Walleij <linus....@linaro.org> wrote:
> On Mon, Jan 25, 2016 at 5:37 PM, Andrew F. Davis <a...@ti.com> wrote:
>
>> Add binding for generic parallel-in/serial-out shift register devices
>> used as GPIO.
>>
>> Signed-off-by: Andrew F. Davis <a...@ti.com>
> (...)
>> +Required properties:
>> + - compatible : Should be "pisosr-gpio".
>
> As mentioned I'd like some vendor examples here with dual strings.
> "ti,foo", "pisosr-gpio"; is fine.

I acked it despite this because I've concluded it is not all that
useful in this case given you can have a variety of number of bits,
can chain together chips, etc. and the opportunity for screwing up or
differentiating this circuit in some way is really outside of the
logic chips themselves. You'd probably want to know the board not the
IC in this case.

The only reason I can think of to know the specific logic chip is if
different versions of IC can be populated and they have different
logic levels requiring regulator programming. We can cross that when
someone adds a supply. Of course, you could just as easily change
supply constraints as compatible strings in that case.

Rob

[Sean Nyekjær]

Please add this :-)
{"ti,sn65hvs885", 0},

> +};
> +MODULE_DEVICE_TABLE(spi, pisosr_gpio_id_table);
> +
> +static const struct of_device_id pisosr_gpio_of_match_table[] = {
> + { .compatible = "pisosr-gpio", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, pisosr_gpio_of_match_table);
> +
> +static struct spi_driver pisosr_gpio_driver = {
> + .driver = {
> + .name = "pisosr-gpio",
> + .of_match_table = pisosr_gpio_of_match_table,
> + },
> + .probe = pisosr_gpio_probe,
> + .remove = pisosr_gpio_remove,
> + .id_table = pisosr_gpio_id_table,
> +};
> +module_spi_driver(pisosr_gpio_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <a...@ti.com>");
> +MODULE_DESCRIPTION("SPI Compatible PISO Shift Register GPIO Driver");
> +MODULE_LICENSE("GPL v2");

Hi Andrew

I have created an driver for the sn65hvs885 at the same time as you, and
the output is nearly identical :-)
We are both missing the support for cacading devices, but i guess that
can come later on.

/Sean

[Andrew F. Davis]

See below.

Hi Sean,

This started as a driver for the sn65hvs88*2*, after seeing other drivers
for similar parts I found that these input shift-register devices interface
mostly the same way, so a more generic catch-all driver might be more
useful.

I would like to avoid explicitly putting the individual part names that may be
compatible in this driver for reasons described well by Rob's comment[0]. If
you disagree I don't have any real serious qualms with adding some part names.

Also by allowing a variable number of bits to be defined in DT I think
cascaded devices should work with this driver already.

[0] https://lkml.org/lkml/2016/1/28/581

Andrew

[Sean Nyekjær]

On 2016-02-01 15:52, Andrew F. Davis wrote:
> On 01/29/2016 12:31 AM, Sean Nyekjær wrote:
>>
>>
>> On 2016-01-25 17:37, Andrew F. Davis wrote:
>>> Add generic parallel-in/serial-out shift register GPIO driver.
>>>
>>> This includes SPI compatible devices like SN74165 serial-out shift
>>> registers and the SN65HVS88x series of industrial serializers that can
>>> be read over the SPI bus and used for GPI (General Purpose Input).
>>>
>>> Signed-off-by: Andrew F. Davis <a...@ti.com>

Tested-by: Sean Nyekjaer <sean.n...@prevas.dk>

Hi Andrew

I have tested this with 2 sn65hvs885 cacaded and i does indeed work :-)

/Sean

[Linus Walleij]

On Mon, Jan 25, 2016 at 5:37 PM, Andrew F. Davis <a...@ti.com> wrote:

> Add generic parallel-in/serial-out shift register GPIO driver.
>
> This includes SPI compatible devices like SN74165 serial-out shift
> registers and the SN65HVS88x series of industrial serializers that can
> be read over the SPI bus and used for GPI (General Purpose Input).
>
> Signed-off-by: Andrew F. Davis <a...@ti.com>

Patch applied, good work.

Yours,
Linus Walleij