* [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver
@ 2017-03-27 6:22 Nandor Han
[not found] ` <cover.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Nandor Han @ 2017-03-27 6:22 UTC (permalink / raw)
To: linus.walleij, gnurou, robh+dt, mark.rutland, linux-gpio,
devicetree, linux-kernel
Cc: Nandor Han
The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
- configure gpin as input/out
- get/set gpio status
Documentation: A gpio-xra1403.txt file was added to document the DTS
bindings related to driver.
Testing:
1. XRA1403 connected to iMX53 MCU
2. Export gpio from userspace
3. Verify that corresponding gpio directories are created
in `/sys/class/gpio/gpioXX`
4. Export gpios from first and second bank as output
5. Set the output gpio pin to high/low and verify with the
oscilloscope that gpio status is according with the configured
value.
Nandor Han (3):
gpio - Add EXAR XRA1403 SPI GPIO expander driver
doc,dts - add XRA1403 DTS binding documentation
Add XRA1403 support to MAINTAINERS file
.../devicetree/bindings/gpio/gpio-xra1403.txt | 37 +++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
MAINTAINERS | 8 +
drivers/gpio/Kconfig | 5 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-xra1403.c | 252 +++++++++++++++++++++
6 files changed, 304 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
create mode 100644 drivers/gpio/gpio-xra1403.c
--
2.10.1
^ permalink raw reply [flat|nested] 17+ messages in thread[parent not found: <cover.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>]
* [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver 2017-03-27 6:22 [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Nandor Han @ 2017-03-27 6:23 ` Nandor Han 2017-03-27 6:23 ` [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation Nandor Han ` (2 subsequent siblings) 3 siblings, 0 replies; 17+ messages in thread From: Nandor Han @ 2017-03-27 6:23 UTC (permalink / raw) To: linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8, linux-gpio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Nandor Han, Semi Malinen This is a simple driver that provides a /sys/class/gpio interface for controlling and configuring the GPIO lines. It does not provide support for chip select or interrupts. Signed-off-by: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org> Signed-off-by: Semi Malinen <semi.malinen-JJi787mZWgc@public.gmane.org> --- .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/gpio/Kconfig | 5 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-xra1403.c | 252 +++++++++++++++++++++ 4 files changed, 259 insertions(+) create mode 100644 drivers/gpio/gpio-xra1403.c diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 0ad67d5..7ca9d41 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -103,6 +103,7 @@ ettus NI Ettus Research eukrea Eukréa Electromatique everest Everest Semiconductor Co. Ltd. everspin Everspin Technologies, Inc. +exar Exar Corporation excito Excito ezchip EZchip Semiconductor faraday Faraday Technology Corporation diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index d6e3cfd..3a6c9a3 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1208,6 +1208,11 @@ config GPIO_PISOSR GPIO driver for SPI compatible parallel-in/serial-out shift registers. These are input only devices. +config GPIO_XRA1403 + tristate "EXAR XRA1403 16-bit GPIO expander" + help + GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander. + endmenu menu "SPI or I2C GPIO expanders" diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index bd995dc..8f50844 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -139,6 +139,7 @@ obj-$(CONFIG_GPIO_XGENE) += gpio-xgene.o obj-$(CONFIG_GPIO_XGENE_SB) += gpio-xgene-sb.o obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o +obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o obj-$(CONFIG_GPIO_ZEVIO) += gpio-zevio.o obj-$(CONFIG_GPIO_ZYNQ) += gpio-zynq.o diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c new file mode 100644 index 0000000..1b7138a8 --- /dev/null +++ b/drivers/gpio/gpio-xra1403.c @@ -0,0 +1,252 @@ +/* + * GPIO driver for EXAR XRA1403 16-bit GPIO expander + * + * Copyright (c) 2017, General Electric Company + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope 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, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> +#include <linux/spi/spi.h> + +/* XRA1403 registers */ +#define XRA_GSR 0x00 /* GPIO State */ +#define XRA_OCR 0x02 /* Output Control */ +#define XRA_GCR 0x06 /* GPIO Configuration */ + +/* SPI headers */ +#define XRA_READ 0x80 /* read bit of the SPI command byte */ + +struct xra1403 { + struct mutex lock; + struct gpio_chip chip; + struct spi_device *spi; +}; + +static int xra1403_get_byte(struct xra1403 *xra, unsigned int addr) +{ + return spi_w8r8(xra->spi, XRA_READ | (addr << 1)); +} + +static int xra1403_get_bit(struct xra1403 *xra, unsigned int addr, + unsigned int bit) +{ + int ret; + + ret = xra1403_get_byte(xra, addr + (bit > 7)); + if (ret < 0) + return ret; + + return !!(ret & BIT(bit % 8)); +} + +static int xra1403_set_bit(struct xra1403 *xra, unsigned int addr, + unsigned int bit, int value) +{ + int ret; + u8 mask; + u8 tx[2]; + + addr += bit > 7; + + mutex_lock(&xra->lock); + + ret = xra1403_get_byte(xra, addr); + if (ret < 0) + goto out_unlock; + + mask = BIT(bit % 8); + if (value) + value = ret | mask; + else + value = ret & ~mask; + + if (value != ret) { + tx[0] = addr << 1; + tx[1] = value; + ret = spi_write(xra->spi, tx, sizeof(tx)); + } else { + ret = 0; + } + +out_unlock: + mutex_unlock(&xra->lock); + + return ret; +} + +static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset) +{ + return xra1403_set_bit(gpiochip_get_data(chip), XRA_GCR, offset, 1); +} + +static int xra1403_direction_output(struct gpio_chip *chip, unsigned int offset, + int value) +{ + int ret; + struct xra1403 *xra = gpiochip_get_data(chip); + + ret = xra1403_set_bit(xra, XRA_OCR, offset, value); + if (ret) + return ret; + + ret = xra1403_set_bit(xra, XRA_GCR, offset, 0); + + return ret; +} + +static int xra1403_get(struct gpio_chip *chip, unsigned int offset) +{ + return xra1403_get_bit(gpiochip_get_data(chip), XRA_GSR, offset); +} + +static void xra1403_set(struct gpio_chip *chip, unsigned int offset, int value) +{ + xra1403_set_bit(gpiochip_get_data(chip), XRA_OCR, offset, value); +} + +#ifdef CONFIG_DEBUG_FS +#define XRA_REGS 0x16 +static void xra1403_dbg_show(struct seq_file *s, struct gpio_chip *chip) +{ + int reg; + struct xra1403 *xra = gpiochip_get_data(chip); + int value[XRA_REGS]; + int i; + unsigned int gcr; + unsigned int gsr; + + seq_puts(s, "xra reg:"); + for (reg = 0; reg < XRA_REGS; reg++) + seq_printf(s, " %2.2x", reg); + seq_puts(s, "\n value:"); + for (reg = 0; reg < XRA_REGS; reg++) { + value[reg] = xra1403_get_byte(xra, reg); + seq_printf(s, " %2.2x", value[reg]); + } + seq_puts(s, "\n"); + + gcr = value[XRA_GCR + 1] << 8 | value[XRA_GCR]; + gsr = value[XRA_GSR + 1] << 8 | value[XRA_GSR]; + for (i = 0; i < chip->ngpio; i++) { + const char *label = gpiochip_is_requested(chip, i); + + if (!label) + continue; + + seq_printf(s, " gpio-%-3d (%-12s) %s %s\n", + chip->base + i, label, + (gcr & BIT(i)) ? "in" : "out", + (gsr & BIT(i)) ? "hi" : "lo"); + } +} +#else +#define xra1403_dbg_show NULL +#endif + +static int xra1403_probe(struct spi_device *spi) +{ + struct xra1403 *xra; + struct gpio_desc *reset_gpio; + + xra = devm_kzalloc(&spi->dev, sizeof(*xra), GFP_KERNEL); + if (!xra) + return -ENOMEM; + + /* bring the chip out of reset */ + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(reset_gpio)) + dev_warn(&spi->dev, "could not get reset-gpios\n"); + else if (reset_gpio) + gpiod_put(reset_gpio); + + mutex_init(&xra->lock); + + xra->chip.direction_input = xra1403_direction_input; + xra->chip.direction_output = xra1403_direction_output; + xra->chip.get = xra1403_get; + xra->chip.set = xra1403_set; + xra->chip.dbg_show = xra1403_dbg_show; + + xra->chip.ngpio = 16; + xra->chip.label = "xra1403"; + + xra->chip.base = -1; + xra->chip.can_sleep = true; + xra->chip.parent = &spi->dev; + xra->chip.owner = THIS_MODULE; + + xra->spi = spi; + spi_set_drvdata(spi, xra); + + return gpiochip_add_data(&xra->chip, xra); +} + +static int xra1403_remove(struct spi_device *spi) +{ + struct xra1403 *xra = spi_get_drvdata(spi); + + gpiochip_remove(&xra->chip); + + return 0; +} + +static const struct spi_device_id xra1403_ids[] = { + { "xra1403" }, + {}, +}; +MODULE_DEVICE_TABLE(spi, xra1403_ids); + +static const struct of_device_id xra1403_spi_of_match[] = { + { .compatible = "exar,xra1403" }, + {}, +}; +MODULE_DEVICE_TABLE(of, xra1403_spi_of_match); + +static struct spi_driver xra1403_driver = { + .probe = xra1403_probe, + .remove = xra1403_remove, + .id_table = xra1403_ids, + .driver = { + .name = "xra1403", + .of_match_table = of_match_ptr(xra1403_spi_of_match), + }, +}; + +static int __init xra1403_init(void) +{ + return spi_register_driver(&xra1403_driver); +} + +/* + * register after spi postcore initcall and before + * subsys initcalls that may rely on these GPIOs + */ +subsys_initcall(xra1403_init); + +static void __exit xra1403_exit(void) +{ + spi_unregister_driver(&xra1403_driver); +} +module_exit(xra1403_exit); + +MODULE_AUTHOR("Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org>"); +MODULE_AUTHOR("Semi Malinen <semi.malinen-JJi787mZWgc@public.gmane.org>"); +MODULE_DESCRIPTION("GPIO expander driver for EXAR XRA1403"); +MODULE_LICENSE("GPL v2"); -- 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver @ 2017-03-27 6:23 ` Nandor Han 0 siblings, 0 replies; 17+ messages in thread From: Nandor Han @ 2017-03-27 6:23 UTC (permalink / raw) To: linus.walleij, gnurou, robh+dt, mark.rutland, linux-gpio, devicetree, linux-kernel Cc: Nandor Han, Semi Malinen This is a simple driver that provides a /sys/class/gpio interface for controlling and configuring the GPIO lines. It does not provide support for chip select or interrupts. Signed-off-by: Nandor Han <nandor.han@ge.com> Signed-off-by: Semi Malinen <semi.malinen@ge.com> --- .../devicetree/bindings/vendor-prefixes.txt | 1 + drivers/gpio/Kconfig | 5 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-xra1403.c | 252 +++++++++++++++++++++ 4 files changed, 259 insertions(+) create mode 100644 drivers/gpio/gpio-xra1403.c diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 0ad67d5..7ca9d41 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -103,6 +103,7 @@ ettus NI Ettus Research eukrea Eukréa Electromatique everest Everest Semiconductor Co. Ltd. everspin Everspin Technologies, Inc. +exar Exar Corporation excito Excito ezchip EZchip Semiconductor faraday Faraday Technology Corporation diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index d6e3cfd..3a6c9a3 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1208,6 +1208,11 @@ config GPIO_PISOSR GPIO driver for SPI compatible parallel-in/serial-out shift registers. These are input only devices. +config GPIO_XRA1403 + tristate "EXAR XRA1403 16-bit GPIO expander" + help + GPIO driver for EXAR XRA1403 16-bit SPI-based GPIO expander. + endmenu menu "SPI or I2C GPIO expanders" diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index bd995dc..8f50844 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -139,6 +139,7 @@ obj-$(CONFIG_GPIO_XGENE) += gpio-xgene.o obj-$(CONFIG_GPIO_XGENE_SB) += gpio-xgene-sb.o obj-$(CONFIG_GPIO_XILINX) += gpio-xilinx.o obj-$(CONFIG_GPIO_XLP) += gpio-xlp.o +obj-$(CONFIG_GPIO_XRA1403) += gpio-xra1403.o obj-$(CONFIG_GPIO_XTENSA) += gpio-xtensa.o obj-$(CONFIG_GPIO_ZEVIO) += gpio-zevio.o obj-$(CONFIG_GPIO_ZYNQ) += gpio-zynq.o diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c new file mode 100644 index 0000000..1b7138a8 --- /dev/null +++ b/drivers/gpio/gpio-xra1403.c @@ -0,0 +1,252 @@ +/* + * GPIO driver for EXAR XRA1403 16-bit GPIO expander + * + * Copyright (c) 2017, General Electric Company + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope 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, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/bitops.h> +#include <linux/gpio/driver.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> +#include <linux/spi/spi.h> + +/* XRA1403 registers */ +#define XRA_GSR 0x00 /* GPIO State */ +#define XRA_OCR 0x02 /* Output Control */ +#define XRA_GCR 0x06 /* GPIO Configuration */ + +/* SPI headers */ +#define XRA_READ 0x80 /* read bit of the SPI command byte */ + +struct xra1403 { + struct mutex lock; + struct gpio_chip chip; + struct spi_device *spi; +}; + +static int xra1403_get_byte(struct xra1403 *xra, unsigned int addr) +{ + return spi_w8r8(xra->spi, XRA_READ | (addr << 1)); +} + +static int xra1403_get_bit(struct xra1403 *xra, unsigned int addr, + unsigned int bit) +{ + int ret; + + ret = xra1403_get_byte(xra, addr + (bit > 7)); + if (ret < 0) + return ret; + + return !!(ret & BIT(bit % 8)); +} + +static int xra1403_set_bit(struct xra1403 *xra, unsigned int addr, + unsigned int bit, int value) +{ + int ret; + u8 mask; + u8 tx[2]; + + addr += bit > 7; + + mutex_lock(&xra->lock); + + ret = xra1403_get_byte(xra, addr); + if (ret < 0) + goto out_unlock; + + mask = BIT(bit % 8); + if (value) + value = ret | mask; + else + value = ret & ~mask; + + if (value != ret) { + tx[0] = addr << 1; + tx[1] = value; + ret = spi_write(xra->spi, tx, sizeof(tx)); + } else { + ret = 0; + } + +out_unlock: + mutex_unlock(&xra->lock); + + return ret; +} + +static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset) +{ + return xra1403_set_bit(gpiochip_get_data(chip), XRA_GCR, offset, 1); +} + +static int xra1403_direction_output(struct gpio_chip *chip, unsigned int offset, + int value) +{ + int ret; + struct xra1403 *xra = gpiochip_get_data(chip); + + ret = xra1403_set_bit(xra, XRA_OCR, offset, value); + if (ret) + return ret; + + ret = xra1403_set_bit(xra, XRA_GCR, offset, 0); + + return ret; +} + +static int xra1403_get(struct gpio_chip *chip, unsigned int offset) +{ + return xra1403_get_bit(gpiochip_get_data(chip), XRA_GSR, offset); +} + +static void xra1403_set(struct gpio_chip *chip, unsigned int offset, int value) +{ + xra1403_set_bit(gpiochip_get_data(chip), XRA_OCR, offset, value); +} + +#ifdef CONFIG_DEBUG_FS +#define XRA_REGS 0x16 +static void xra1403_dbg_show(struct seq_file *s, struct gpio_chip *chip) +{ + int reg; + struct xra1403 *xra = gpiochip_get_data(chip); + int value[XRA_REGS]; + int i; + unsigned int gcr; + unsigned int gsr; + + seq_puts(s, "xra reg:"); + for (reg = 0; reg < XRA_REGS; reg++) + seq_printf(s, " %2.2x", reg); + seq_puts(s, "\n value:"); + for (reg = 0; reg < XRA_REGS; reg++) { + value[reg] = xra1403_get_byte(xra, reg); + seq_printf(s, " %2.2x", value[reg]); + } + seq_puts(s, "\n"); + + gcr = value[XRA_GCR + 1] << 8 | value[XRA_GCR]; + gsr = value[XRA_GSR + 1] << 8 | value[XRA_GSR]; + for (i = 0; i < chip->ngpio; i++) { + const char *label = gpiochip_is_requested(chip, i); + + if (!label) + continue; + + seq_printf(s, " gpio-%-3d (%-12s) %s %s\n", + chip->base + i, label, + (gcr & BIT(i)) ? "in" : "out", + (gsr & BIT(i)) ? "hi" : "lo"); + } +} +#else +#define xra1403_dbg_show NULL +#endif + +static int xra1403_probe(struct spi_device *spi) +{ + struct xra1403 *xra; + struct gpio_desc *reset_gpio; + + xra = devm_kzalloc(&spi->dev, sizeof(*xra), GFP_KERNEL); + if (!xra) + return -ENOMEM; + + /* bring the chip out of reset */ + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(reset_gpio)) + dev_warn(&spi->dev, "could not get reset-gpios\n"); + else if (reset_gpio) + gpiod_put(reset_gpio); + + mutex_init(&xra->lock); + + xra->chip.direction_input = xra1403_direction_input; + xra->chip.direction_output = xra1403_direction_output; + xra->chip.get = xra1403_get; + xra->chip.set = xra1403_set; + xra->chip.dbg_show = xra1403_dbg_show; + + xra->chip.ngpio = 16; + xra->chip.label = "xra1403"; + + xra->chip.base = -1; + xra->chip.can_sleep = true; + xra->chip.parent = &spi->dev; + xra->chip.owner = THIS_MODULE; + + xra->spi = spi; + spi_set_drvdata(spi, xra); + + return gpiochip_add_data(&xra->chip, xra); +} + +static int xra1403_remove(struct spi_device *spi) +{ + struct xra1403 *xra = spi_get_drvdata(spi); + + gpiochip_remove(&xra->chip); + + return 0; +} + +static const struct spi_device_id xra1403_ids[] = { + { "xra1403" }, + {}, +}; +MODULE_DEVICE_TABLE(spi, xra1403_ids); + +static const struct of_device_id xra1403_spi_of_match[] = { + { .compatible = "exar,xra1403" }, + {}, +}; +MODULE_DEVICE_TABLE(of, xra1403_spi_of_match); + +static struct spi_driver xra1403_driver = { + .probe = xra1403_probe, + .remove = xra1403_remove, + .id_table = xra1403_ids, + .driver = { + .name = "xra1403", + .of_match_table = of_match_ptr(xra1403_spi_of_match), + }, +}; + +static int __init xra1403_init(void) +{ + return spi_register_driver(&xra1403_driver); +} + +/* + * register after spi postcore initcall and before + * subsys initcalls that may rely on these GPIOs + */ +subsys_initcall(xra1403_init); + +static void __exit xra1403_exit(void) +{ + spi_unregister_driver(&xra1403_driver); +} +module_exit(xra1403_exit); + +MODULE_AUTHOR("Nandor Han <nandor.han@ge.com>"); +MODULE_AUTHOR("Semi Malinen <semi.malinen@ge.com>"); +MODULE_DESCRIPTION("GPIO expander driver for EXAR XRA1403"); +MODULE_LICENSE("GPL v2"); -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <6bf04ba1761f0692cb461558f0c8836f0d1f7ad8.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>]
* Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver 2017-03-27 6:23 ` Nandor Han @ 2017-03-29 2:07 ` Linus Walleij -1 siblings, 0 replies; 17+ messages in thread From: Linus Walleij @ 2017-03-29 2:07 UTC (permalink / raw) To: Nandor Han Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Semi Malinen On Mon, Mar 27, 2017 at 8:23 AM, Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org> wrote: > This is a simple driver that provides a /sys/class/gpio > interface for controlling and configuring the GPIO lines. Use the gpio tools in tools/gpio, use the characcter device. Do not use sysfs. Change this to reference the tools. > It does not provide support for chip select or interrupts. > > Signed-off-by: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org> > Signed-off-by: Semi Malinen <semi.malinen-JJi787mZWgc@public.gmane.org> (...) > +exar Exar Corporation Send this as a separate patch to the DT bindings maintainer (Rob Herring.) > +static int xra1403_get_byte(struct xra1403 *xra, unsigned int addr) > +{ > + return spi_w8r8(xra->spi, XRA_READ | (addr << 1)); > +} > + > +static int xra1403_get_bit(struct xra1403 *xra, unsigned int addr, > + unsigned int bit) > +{ > + int ret; > + > + ret = xra1403_get_byte(xra, addr + (bit > 7)); > + if (ret < 0) > + return ret; > + > + return !!(ret & BIT(bit % 8)); > +} This looks like it can use regmap-spi right off, do you agree? git grep devm_regmap_init_spi should give you some examples of how to use it. If it's not off-the shelf regmap drivers like drivers/iio/pressure/mpl115_spi.c give examples of how to make more elaborate custom SPI transfers with regmap. > +static int xra1403_set_bit(struct xra1403 *xra, unsigned int addr, > + unsigned int bit, int value) > +{ > + int ret; > + u8 mask; > + u8 tx[2]; > + > + addr += bit > 7; > + > + mutex_lock(&xra->lock); > + > + ret = xra1403_get_byte(xra, addr); > + if (ret < 0) > + goto out_unlock; > + > + mask = BIT(bit % 8); > + if (value) > + value = ret | mask; > + else > + value = ret & ~mask; > + > + if (value != ret) { > + tx[0] = addr << 1; > + tx[1] = value; > + ret = spi_write(xra->spi, tx, sizeof(tx)); > + } else { > + ret = 0; > + } > + > +out_unlock: > + mutex_unlock(&xra->lock); > + > + return ret; > +} Classical mask-and-set implementation right? With regmap this becomes simply regmap_update_bits(map, addr, mask, set) > +static int xra1403_probe(struct spi_device *spi) > +{ > + struct xra1403 *xra; > + struct gpio_desc *reset_gpio; > + > + xra = devm_kzalloc(&spi->dev, sizeof(*xra), GFP_KERNEL); > + if (!xra) > + return -ENOMEM; > + > + /* bring the chip out of reset */ > + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(reset_gpio)) > + dev_warn(&spi->dev, "could not get reset-gpios\n"); > + else if (reset_gpio) > + gpiod_put(reset_gpio); I don't think you should put it, other than in the remove() function and in that case you need to have it in the state container. > + mutex_init(&xra->lock); > + > + xra->chip.direction_input = xra1403_direction_input; > + xra->chip.direction_output = xra1403_direction_output; Please implement .get_direction(). This is very nice to have. > +static int xra1403_remove(struct spi_device *spi) > +{ > + struct xra1403 *xra = spi_get_drvdata(spi); > + > + gpiochip_remove(&xra->chip); Use devm_gpiochip_add_data() and this remove is not needed at all. > +static int __init xra1403_init(void) > +{ > + return spi_register_driver(&xra1403_driver); > +} > + > +/* > + * register after spi postcore initcall and before > + * subsys initcalls that may rely on these GPIOs > + */ > +subsys_initcall(xra1403_init); > + > +static void __exit xra1403_exit(void) > +{ > + spi_unregister_driver(&xra1403_driver); > +} > +module_exit(xra1403_exit); This seems like tricksy. Just module_spi_driver() should be fine don't you think? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver @ 2017-03-29 2:07 ` Linus Walleij 0 siblings, 0 replies; 17+ messages in thread From: Linus Walleij @ 2017-03-29 2:07 UTC (permalink / raw) To: Nandor Han Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel, Semi Malinen On Mon, Mar 27, 2017 at 8:23 AM, Nandor Han <nandor.han@ge.com> wrote: > This is a simple driver that provides a /sys/class/gpio > interface for controlling and configuring the GPIO lines. Use the gpio tools in tools/gpio, use the characcter device. Do not use sysfs. Change this to reference the tools. > It does not provide support for chip select or interrupts. > > Signed-off-by: Nandor Han <nandor.han@ge.com> > Signed-off-by: Semi Malinen <semi.malinen@ge.com> (...) > +exar Exar Corporation Send this as a separate patch to the DT bindings maintainer (Rob Herring.) > +static int xra1403_get_byte(struct xra1403 *xra, unsigned int addr) > +{ > + return spi_w8r8(xra->spi, XRA_READ | (addr << 1)); > +} > + > +static int xra1403_get_bit(struct xra1403 *xra, unsigned int addr, > + unsigned int bit) > +{ > + int ret; > + > + ret = xra1403_get_byte(xra, addr + (bit > 7)); > + if (ret < 0) > + return ret; > + > + return !!(ret & BIT(bit % 8)); > +} This looks like it can use regmap-spi right off, do you agree? git grep devm_regmap_init_spi should give you some examples of how to use it. If it's not off-the shelf regmap drivers like drivers/iio/pressure/mpl115_spi.c give examples of how to make more elaborate custom SPI transfers with regmap. > +static int xra1403_set_bit(struct xra1403 *xra, unsigned int addr, > + unsigned int bit, int value) > +{ > + int ret; > + u8 mask; > + u8 tx[2]; > + > + addr += bit > 7; > + > + mutex_lock(&xra->lock); > + > + ret = xra1403_get_byte(xra, addr); > + if (ret < 0) > + goto out_unlock; > + > + mask = BIT(bit % 8); > + if (value) > + value = ret | mask; > + else > + value = ret & ~mask; > + > + if (value != ret) { > + tx[0] = addr << 1; > + tx[1] = value; > + ret = spi_write(xra->spi, tx, sizeof(tx)); > + } else { > + ret = 0; > + } > + > +out_unlock: > + mutex_unlock(&xra->lock); > + > + return ret; > +} Classical mask-and-set implementation right? With regmap this becomes simply regmap_update_bits(map, addr, mask, set) > +static int xra1403_probe(struct spi_device *spi) > +{ > + struct xra1403 *xra; > + struct gpio_desc *reset_gpio; > + > + xra = devm_kzalloc(&spi->dev, sizeof(*xra), GFP_KERNEL); > + if (!xra) > + return -ENOMEM; > + > + /* bring the chip out of reset */ > + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); > + if (IS_ERR(reset_gpio)) > + dev_warn(&spi->dev, "could not get reset-gpios\n"); > + else if (reset_gpio) > + gpiod_put(reset_gpio); I don't think you should put it, other than in the remove() function and in that case you need to have it in the state container. > + mutex_init(&xra->lock); > + > + xra->chip.direction_input = xra1403_direction_input; > + xra->chip.direction_output = xra1403_direction_output; Please implement .get_direction(). This is very nice to have. > +static int xra1403_remove(struct spi_device *spi) > +{ > + struct xra1403 *xra = spi_get_drvdata(spi); > + > + gpiochip_remove(&xra->chip); Use devm_gpiochip_add_data() and this remove is not needed at all. > +static int __init xra1403_init(void) > +{ > + return spi_register_driver(&xra1403_driver); > +} > + > +/* > + * register after spi postcore initcall and before > + * subsys initcalls that may rely on these GPIOs > + */ > +subsys_initcall(xra1403_init); > + > +static void __exit xra1403_exit(void) > +{ > + spi_unregister_driver(&xra1403_driver); > +} > +module_exit(xra1403_exit); This seems like tricksy. Just module_spi_driver() should be fine don't you think? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver 2017-03-29 2:07 ` Linus Walleij (?) @ 2017-04-05 13:24 ` Han, Nandor (GE Healthcare) 2017-04-07 10:07 ` Linus Walleij -1 siblings, 1 reply; 17+ messages in thread From: Han, Nandor (GE Healthcare) @ 2017-04-05 13:24 UTC (permalink / raw) To: Linus Walleij Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel, Malinen, Semi (GE Healthcare) > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@linaro.org] > Sent: 29 March 2017 05:07 > To: Han, Nandor (GE Healthcare) <nandor.han@ge.com> > Cc: Alexandre Courbot <gnurou@gmail.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland > <mark.rutland@arm.com>; linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > Malinen, Semi (GE Healthcare) <semi.malinen@ge.com> > Subject: EXT: Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver > > On Mon, Mar 27, 2017 at 8:23 AM, Nandor Han <nandor.han@ge.com> wrote: > > > This is a simple driver that provides a /sys/class/gpio > > interface for controlling and configuring the GPIO lines. > > Use the gpio tools in tools/gpio, use the characcter device. > Do not use sysfs. Change this to reference the tools. > > > It does not provide support for chip select or interrupts. > > > > Signed-off-by: Nandor Han <nandor.han@ge.com> > > Signed-off-by: Semi Malinen <semi.malinen@ge.com> > (...) > > +exar Exar Corporation > > Send this as a separate patch to the DT bindings maintainer > (Rob Herring.) > OK. I will create a separate patch with this one. I guess is not an issue to send all the patches to Rob as well. <snip> > > + > > + ret = xra1403_get_byte(xra, addr + (bit > 7)); > > + if (ret < 0) > > + return ret; > > + > > + return !!(ret & BIT(bit % 8)); > > +} > > This looks like it can use regmap-spi right off, do you agree? > Yes. Using regmap-spi will definitely improve the code readability and reduce boilerplate. Done. > git grep devm_regmap_init_spi > should give you some examples of how to use it. > > If it's not off-the shelf regmap drivers like > drivers/iio/pressure/mpl115_spi.c > give examples of how to make more elaborate custom > SPI transfers with regmap. > Thanks, I did check other drivers as examples. Not that I needed for this driver, but ...mpl115_spi.c doesn't seem to use regmap (checked on next-20170327) <snip> > > + > > + if (value != ret) { > > + tx[0] = addr << 1; > > + tx[1] = value; > > + ret = spi_write(xra->spi, tx, sizeof(tx)); > > + } else { > > + ret = 0; > > + } > > + > > +out_unlock: > > + mutex_unlock(&xra->lock); > > + > > + return ret; > > +} > > Classical mask-and-set implementation right? > With regmap this becomes simply regmap_update_bits(map, addr, mask, set) > True. :) <snip> > > + /* bring the chip out of reset */ > > + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); > > + if (IS_ERR(reset_gpio)) > > + dev_warn(&spi->dev, "could not get reset-gpios\n"); > > + else if (reset_gpio) > > + gpiod_put(reset_gpio); > > I don't think you should put it, other than in the remove() > function and in that case you need to have it in the > state container. Can you please be more explicit here. Currently I'm trying to bring the device out from reset in case reset GPIO is provided. I don't see how this could be done in remove() :) > > > + mutex_init(&xra->lock); > > + > > + xra->chip.direction_input = xra1403_direction_input; > > + xra->chip.direction_output = xra1403_direction_output; > > Please implement .get_direction(). This is very nice to have. > Done > > +static int xra1403_remove(struct spi_device *spi) > > +{ > > + struct xra1403 *xra = spi_get_drvdata(spi); > > + > > + gpiochip_remove(&xra->chip); > > Use devm_gpiochip_add_data() and this remove is not > needed at all. > True. Done <snip> > > +subsys_initcall(xra1403_init); > > + > > +static void __exit xra1403_exit(void) > > +{ > > + spi_unregister_driver(&xra1403_driver); > > +} > > +module_exit(xra1403_exit); > > This seems like tricksy. Just module_spi_driver() > should be fine don't you think? Yeah. TBH I don't have a strong reason why module_spi_driver init level shouldn't be enough. Done. Regards, Nandor ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver 2017-04-05 13:24 ` Han, Nandor (GE Healthcare) @ 2017-04-07 10:07 ` Linus Walleij [not found] ` <CACRpkdYZVy2t9=K0jVm5=BxhWWUwiNsGDTMVBbSiB9+1+uafJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-04-07 10:07 UTC (permalink / raw) To: Han, Nandor (GE Healthcare) Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel, Malinen, Semi (GE Healthcare) On Wed, Apr 5, 2017 at 3:24 PM, Han, Nandor (GE Healthcare) <nandor.han@ge.com> wrote: > [Me] >> > + /* bring the chip out of reset */ >> > + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); >> > + if (IS_ERR(reset_gpio)) >> > + dev_warn(&spi->dev, "could not get reset-gpios\n"); >> > + else if (reset_gpio) >> > + gpiod_put(reset_gpio); >> >> I don't think you should put it, other than in the remove() >> function and in that case you need to have it in the >> state container. > > Can you please be more explicit here. > > Currently I'm trying to bring the device out from reset in case reset GPIO is provided. > I don't see how this could be done in remove() :) If you issue gpiod_put() you release the GPIO hande so something else can go in and grab the GPIO and assert the reset. This is not what you want to make possible: you want to hold this gpiod handle as long as the driver is running. devm_gpiod_get_optional() will do the trick if you don't want to put the handle under explicit control. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CACRpkdYZVy2t9=K0jVm5=BxhWWUwiNsGDTMVBbSiB9+1+uafJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver 2017-04-07 10:07 ` Linus Walleij @ 2017-04-08 7:38 ` Han, Nandor (GE Healthcare) 0 siblings, 0 replies; 17+ messages in thread From: Han, Nandor (GE Healthcare) @ 2017-04-08 7:38 UTC (permalink / raw) To: Linus Walleij Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Malinen, Semi (GE Healthcare) [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2429 bytes --] > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@linaro.org] > Sent: 07 April 2017 13:07 > To: Han, Nandor (GE Healthcare) <nandor.han@ge.com> > Cc: Alexandre Courbot <gnurou@gmail.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland > <mark.rutland@arm.com>; linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > Malinen, Semi (GE Healthcare) <semi.malinen@ge.com> > Subject: EXT: Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver > > On Wed, Apr 5, 2017 at 3:24 PM, Han, Nandor (GE Healthcare) > <nandor.han@ge.com> wrote: > > [Me] > >> > + /* bring the chip out of reset */ > >> > + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); > >> > + if (IS_ERR(reset_gpio)) > >> > + dev_warn(&spi->dev, "could not get reset-gpios\n"); > >> > + else if (reset_gpio) > >> > + gpiod_put(reset_gpio); > >> > >> I don't think you should put it, other than in the remove() > >> function and in that case you need to have it in the > >> state container. > > > > Can you please be more explicit here. > > > > Currently I'm trying to bring the device out from reset in case reset GPIO is provided. > > I don't see how this could be done in remove() :) > > If you issue gpiod_put() you release the GPIO hande so something else > can go in and grab the GPIO and assert the reset. > > This is not what you want to make possible: you want to hold this gpiod handle > as long as the driver is running. devm_gpiod_get_optional() will do the > trick if you don't want to put the handle under explicit control. > That was my first intention to release the reset line in case somebody else wants to control it. I did it like that because usually reset line controls multiple devices and probably some upper layer wants to control that. After your comment I did some analysing and I will follow your advice and change the reset line handling. Once the GPIO is provided to the driver the driver will own it and bring out the device from reset. In case not provided the reset line is somebody else responsibility. This way we are able to cover multiple use-cases. Thanks Linus, Nandy > Yours, > Linus Walleij N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver @ 2017-04-08 7:38 ` Han, Nandor (GE Healthcare) 0 siblings, 0 replies; 17+ messages in thread From: Han, Nandor (GE Healthcare) @ 2017-04-08 7:38 UTC (permalink / raw) To: Linus Walleij Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel, Malinen, Semi (GE Healthcare) > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@linaro.org] > Sent: 07 April 2017 13:07 > To: Han, Nandor (GE Healthcare) <nandor.han@ge.com> > Cc: Alexandre Courbot <gnurou@gmail.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland > <mark.rutland@arm.com>; linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > Malinen, Semi (GE Healthcare) <semi.malinen@ge.com> > Subject: EXT: Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver > > On Wed, Apr 5, 2017 at 3:24 PM, Han, Nandor (GE Healthcare) > <nandor.han@ge.com> wrote: > > [Me] > >> > + /* bring the chip out of reset */ > >> > + reset_gpio = gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW); > >> > + if (IS_ERR(reset_gpio)) > >> > + dev_warn(&spi->dev, "could not get reset-gpios\n"); > >> > + else if (reset_gpio) > >> > + gpiod_put(reset_gpio); > >> > >> I don't think you should put it, other than in the remove() > >> function and in that case you need to have it in the > >> state container. > > > > Can you please be more explicit here. > > > > Currently I'm trying to bring the device out from reset in case reset GPIO is provided. > > I don't see how this could be done in remove() :) > > If you issue gpiod_put() you release the GPIO hande so something else > can go in and grab the GPIO and assert the reset. > > This is not what you want to make possible: you want to hold this gpiod handle > as long as the driver is running. devm_gpiod_get_optional() will do the > trick if you don't want to put the handle under explicit control. > That was my first intention to release the reset line in case somebody else wants to control it. I did it like that because usually reset line controls multiple devices and probably some upper layer wants to control that. After your comment I did some analysing and I will follow your advice and change the reset line handling. Once the GPIO is provided to the driver the driver will own it and bring out the device from reset. In case not provided the reset line is somebody else responsibility. This way we are able to cover multiple use-cases. Thanks Linus, Nandy > Yours, > Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO expander driver 2017-03-27 6:23 ` Nandor Han (?) (?) @ 2017-03-31 12:38 ` Rob Herring -1 siblings, 0 replies; 17+ messages in thread From: Rob Herring @ 2017-03-31 12:38 UTC (permalink / raw) To: Nandor Han Cc: linus.walleij, gnurou, mark.rutland, linux-gpio, devicetree, linux-kernel, Semi Malinen On Mon, Mar 27, 2017 at 09:23:00AM +0300, Nandor Han wrote: > This is a simple driver that provides a /sys/class/gpio > interface for controlling and configuring the GPIO lines. > It does not provide support for chip select or interrupts. > > Signed-off-by: Nandor Han <nandor.han@ge.com> > Signed-off-by: Semi Malinen <semi.malinen@ge.com> > --- > .../devicetree/bindings/vendor-prefixes.txt | 1 + This should go with the binding patch or by itself. > drivers/gpio/Kconfig | 5 + > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-xra1403.c | 252 +++++++++++++++++++++ > 4 files changed, 259 insertions(+) > create mode 100644 drivers/gpio/gpio-xra1403.c ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation 2017-03-27 6:22 [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Nandor Han [not found] ` <cover.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org> @ 2017-03-27 6:23 ` Nandor Han [not found] ` <39ba92bacf48da957f8f85d5cb11d3254fe3d68f.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org> 2017-03-31 18:49 ` Rob Herring 2017-03-27 6:23 ` [PATCH 3/3] Add XRA1403 support to MAINTAINERS file Nandor Han 2017-03-29 1:50 ` [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Linus Walleij 3 siblings, 2 replies; 17+ messages in thread From: Nandor Han @ 2017-03-27 6:23 UTC (permalink / raw) To: linus.walleij, gnurou, robh+dt, mark.rutland, linux-gpio, devicetree, linux-kernel Cc: Nandor Han Add the XRA1403 DTS binding documentation. Signed-off-by: Nandor Han <nandor.han@ge.com> --- .../devicetree/bindings/gpio/gpio-xra1403.txt | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt diff --git a/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt new file mode 100644 index 0000000..ccf5337 --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt @@ -0,0 +1,37 @@ +GPIO Driver for XRA1403 16-BIT GPIO Expander With Reset Input from EXAR + +The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features available: + - Individually programmable inputs: + - Internal pull-up resistors + - Polarity inversion + - Individual interrupt enable + - Rising edge and/or Falling edge interrupt + - Input filter + - Individually programmable outputs + - Output Level Control + - Output Three-State Control + +Properties +---------- +Check documentation for SPI and GPIO controllers regarding properties needed to configure the node. + + - compatible = "exar,xra1403". + - reg = SPI id of the device. + - gpio-controller: mark the node as gpio. + +Optional properties: +------------------- + - reset-gpios: in case available used to control the device reset line. + +Example +-------- + + gpioxra0: gpio@2 { + compatible = "exar,xra1403"; + reg = <2>; + gpio-controller; + #gpio-cells = <2>; + reset-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>; + spi-max-frequency = <1000000>; + status = "okay"; + }; -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
[parent not found: <39ba92bacf48da957f8f85d5cb11d3254fe3d68f.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>]
* Re: [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation 2017-03-27 6:23 ` [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation Nandor Han @ 2017-03-29 2:09 ` Linus Walleij 2017-03-31 18:49 ` Rob Herring 1 sibling, 0 replies; 17+ messages in thread From: Linus Walleij @ 2017-03-29 2:09 UTC (permalink / raw) To: Nandor Han Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Mon, Mar 27, 2017 at 8:23 AM, Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org> wrote: > Add the XRA1403 DTS binding documentation. > > Signed-off-by: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org> There is no big problem with this but: > +The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features available: > + - Individually programmable inputs: > + - Internal pull-up resistors > + - Polarity inversion > + - Individual interrupt enable > + - Rising edge and/or Falling edge interrupt > + - Input filter Since you mention that it has interrupts maybe you want to add bindings for the cascaded interrupt and the interrupt-controller; keyword etc already now. We just document what the hardware can do, we don't have to do all of it in the first Linux driver submission. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation @ 2017-03-29 2:09 ` Linus Walleij 0 siblings, 0 replies; 17+ messages in thread From: Linus Walleij @ 2017-03-29 2:09 UTC (permalink / raw) To: Nandor Han Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel On Mon, Mar 27, 2017 at 8:23 AM, Nandor Han <nandor.han@ge.com> wrote: > Add the XRA1403 DTS binding documentation. > > Signed-off-by: Nandor Han <nandor.han@ge.com> There is no big problem with this but: > +The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features available: > + - Individually programmable inputs: > + - Internal pull-up resistors > + - Polarity inversion > + - Individual interrupt enable > + - Rising edge and/or Falling edge interrupt > + - Input filter Since you mention that it has interrupts maybe you want to add bindings for the cascaded interrupt and the interrupt-controller; keyword etc already now. We just document what the hardware can do, we don't have to do all of it in the first Linux driver submission. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation 2017-03-27 6:23 ` [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation Nandor Han [not found] ` <39ba92bacf48da957f8f85d5cb11d3254fe3d68f.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org> @ 2017-03-31 18:49 ` Rob Herring 1 sibling, 0 replies; 17+ messages in thread From: Rob Herring @ 2017-03-31 18:49 UTC (permalink / raw) To: Nandor Han Cc: linus.walleij, gnurou, mark.rutland, linux-gpio, devicetree, linux-kernel On Mon, Mar 27, 2017 at 09:23:01AM +0300, Nandor Han wrote: > Add the XRA1403 DTS binding documentation. "dt-bindings: gpio: ..." for the subject prefix please. > > Signed-off-by: Nandor Han <nandor.han@ge.com> > --- > .../devicetree/bindings/gpio/gpio-xra1403.txt | 37 ++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt > > diff --git a/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt > new file mode 100644 > index 0000000..ccf5337 > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt > @@ -0,0 +1,37 @@ > +GPIO Driver for XRA1403 16-BIT GPIO Expander With Reset Input from EXAR > + > +The XRA1403 is an 16-bit GPIO expander with an SPI interface. Features available: > + - Individually programmable inputs: > + - Internal pull-up resistors > + - Polarity inversion > + - Individual interrupt enable > + - Rising edge and/or Falling edge interrupt > + - Input filter > + - Individually programmable outputs > + - Output Level Control > + - Output Three-State Control > + > +Properties > +---------- > +Check documentation for SPI and GPIO controllers regarding properties needed to configure the node. > + > + - compatible = "exar,xra1403". > + - reg = SPI id of the device. > + - gpio-controller: mark the node as gpio. #gpio-cells? > + > +Optional properties: > +------------------- > + - reset-gpios: in case available used to control the device reset line. > + > +Example > +-------- > + > + gpioxra0: gpio@2 { > + compatible = "exar,xra1403"; > + reg = <2>; > + gpio-controller; > + #gpio-cells = <2>; > + reset-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>; > + spi-max-frequency = <1000000>; > + status = "okay"; Don't show status in examples. > + }; > -- > 2.10.1 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] Add XRA1403 support to MAINTAINERS file 2017-03-27 6:22 [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Nandor Han [not found] ` <cover.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org> 2017-03-27 6:23 ` [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation Nandor Han @ 2017-03-27 6:23 ` Nandor Han 2017-03-29 1:50 ` [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Linus Walleij 3 siblings, 0 replies; 17+ messages in thread From: Nandor Han @ 2017-03-27 6:23 UTC (permalink / raw) To: linus.walleij, gnurou, robh+dt, mark.rutland, linux-gpio, devicetree, linux-kernel Cc: Nandor Han Add the XRA1403 support to MAINTAINERS list. Signed-off-by: Nandor Han <nandor.han@ge.com> --- MAINTAINERS | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 58b3a22..539c88c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13903,6 +13903,14 @@ L: linux-kernel@vger.kernel.org S: Supported F: drivers/char/xillybus/ +XRA1403 GPIO EXPANDER +M: Nandor Han <nandor.han@ge.com> +M: Semi Malinen <semi.malinen@ge.com> +L: linux-gpio@vger.kernel.org +S: Maintained +F: drivers/gpio/gpio-xra1403.c +F: Documentation/devicetree/bindings/gpio/gpio-xra1403.txt + XTENSA XTFPGA PLATFORM SUPPORT M: Max Filippov <jcmvbkbc@gmail.com> L: linux-xtensa@linux-xtensa.org -- 2.10.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver 2017-03-27 6:22 [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Nandor Han ` (2 preceding siblings ...) 2017-03-27 6:23 ` [PATCH 3/3] Add XRA1403 support to MAINTAINERS file Nandor Han @ 2017-03-29 1:50 ` Linus Walleij 2017-04-05 12:46 ` Han, Nandor (GE Healthcare) 3 siblings, 1 reply; 17+ messages in thread From: Linus Walleij @ 2017-03-29 1:50 UTC (permalink / raw) To: Nandor Han Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel On Mon, Mar 27, 2017 at 8:22 AM, Nandor Han <nandor.han@ge.com> wrote: > Testing: > > 1. XRA1403 connected to iMX53 MCU > 2. Export gpio from userspace > 3. Verify that corresponding gpio directories are created > in `/sys/class/gpio/gpioXX` > 4. Export gpios from first and second bank as output > 5. Set the output gpio pin to high/low and verify with the > oscilloscope that gpio status is according with the configured > value. Do *NOT* use the sysfs for testing GPIO. This is being phased out. Use the tools in tools/gpio/* so that you exercise the character device instead of the old deprecated ABI. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver 2017-03-29 1:50 ` [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Linus Walleij @ 2017-04-05 12:46 ` Han, Nandor (GE Healthcare) 0 siblings, 0 replies; 17+ messages in thread From: Han, Nandor (GE Healthcare) @ 2017-04-05 12:46 UTC (permalink / raw) To: Linus Walleij Cc: Alexandre Courbot, Rob Herring, Mark Rutland, linux-gpio, devicetree, linux-kernel > -----Original Message----- > From: Linus Walleij [mailto:linus.walleij@linaro.org] > Sent: 29 March 2017 04:51 > To: Han, Nandor (GE Healthcare) <nandor.han@ge.com> > Cc: Alexandre Courbot <gnurou@gmail.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland > <mark.rutland@arm.com>; linux-gpio@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: EXT: Re: [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver > <snip> > Do *NOT* use the sysfs for testing GPIO. > This is being phased out. > > Use the tools in tools/gpio/* so that you exercise the > character device instead of the old deprecated ABI. > Thanks Linus, really good and helpful comments. I agree that make more sense to test with tools/gpio/*. > Yours, > Linus Walleij ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2017-04-08 7:39 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 6:22 [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Nandor Han
[not found] ` <cover.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>
2017-03-27 6:23 ` [PATCH 1/3] gpio - Add EXAR XRA1403 SPI GPIO " Nandor Han
2017-03-27 6:23 ` Nandor Han
[not found] ` <6bf04ba1761f0692cb461558f0c8836f0d1f7ad8.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>
2017-03-29 2:07 ` Linus Walleij
2017-03-29 2:07 ` Linus Walleij
2017-04-05 13:24 ` Han, Nandor (GE Healthcare)
2017-04-07 10:07 ` Linus Walleij
[not found] ` <CACRpkdYZVy2t9=K0jVm5=BxhWWUwiNsGDTMVBbSiB9+1+uafJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-08 7:38 ` Han, Nandor (GE Healthcare)
2017-04-08 7:38 ` Han, Nandor (GE Healthcare)
2017-03-31 12:38 ` Rob Herring
2017-03-27 6:23 ` [PATCH 2/3] doc,dts - add XRA1403 DTS binding documentation Nandor Han
[not found] ` <39ba92bacf48da957f8f85d5cb11d3254fe3d68f.1490595641.git.nandor.han-JJi787mZWgc@public.gmane.org>
2017-03-29 2:09 ` Linus Walleij
2017-03-29 2:09 ` Linus Walleij
2017-03-31 18:49 ` Rob Herring
2017-03-27 6:23 ` [PATCH 3/3] Add XRA1403 support to MAINTAINERS file Nandor Han
2017-03-29 1:50 ` [PATCH 0/3] XRA1403,gpio - add XRA1403 gpio expander driver Linus Walleij
2017-04-05 12:46 ` Han, Nandor (GE Healthcare)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.