[PATCH v2 0/4] XRA1403,gpio - add XRA1403 gpio expander driver

[PATCH v2 0/4] XRA1403,gpio - add XRA1403 gpio expander driver

[Nandor Han]

The patchset will add a driver to support basic functionality for
XRA1403 device. Features supported:
        - get/set GPIO direction (input, output)
        - get/set GPIO level (low, high)

Documentation: A gpio-xra1403.txt file was added to document the DTS
	bindings related to driver.

Testing:

1.1 XRA1403 connected to iMX53 MCU
1.2 Use the GPIO tools provided by kernel from tools/gpio dir

2.1 `lsgpio`

root@csmon ppd:~# lsgpio
GPIO chip: gpiochip8, "xra1403", 16 GPIO lines
        line  0: unnamed unused [output]
        line  1: unnamed unused [output]
        line  2: unnamed unused [output]
        line  3: unnamed unused [output]
        line  4: unnamed unused [output]
        line  5: unnamed unused
        line  6: unnamed unused
        line  7: unnamed unused [output]
        line  8: unnamed unused [output]
        line  9: unnamed unused
        line 10: unnamed unused [output]
        line 11: unnamed unused
        line 12: unnamed unused
        line 13: unnamed unused
        line 14: unnamed unused
        line 15: unnamed unused [output]
GPIO chip: gpiochip7, "xra1403", 16 GPIO lines
        line  0: unnamed unused
        line  1: unnamed unused
        line  2: unnamed unused
        line  3: unnamed unused
        line  4: unnamed unused
        line  5: unnamed unused
        line  6: unnamed unused
        line  7: unnamed unused
        line  8: unnamed unused
        line  9: unnamed unused
        line 10: unnamed unused
        line 11: unnamed unused
        line 12: unnamed unused
        line 13: unnamed unused
        line 14: unnamed unused
        line 15: unnamed unused

3.1 `gpio-hammer`

root@csmon ppd:~# gpio-hammer -n gpiochip8 -o0 -o1 -o2 -o3
Hammer lines [0, 1, 2, 3] on gpiochip8, initial states: [0, 0, 0, 0]
[\] [0: 0, 1: 0, 2: 0, 3: 0]
...
[\] [0: 1, 1: 1, 2: 1, 3: 1]

When using `gpio-hammer` I also attached an oscilloscope to one of the pins and I was
able to monitor and validate the GPIO status.

Nandor Han (4):
  dt-bindings: gpio - add exar to vendor prefixes list
  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      |  46 ++++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 MAINTAINERS                                        |   8 +
 drivers/gpio/Kconfig                               |   5 +
 drivers/gpio/Makefile                              |   1 +
 drivers/gpio/gpio-xra1403.c                        | 236 +++++++++++++++++++++
 6 files changed, 297 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
 create mode 100644 drivers/gpio/gpio-xra1403.c

-- 
2.10.1

[PATCH v2 1/4] dt-bindings: gpio - add exar to vendor prefixes list

[Nandor Han]

Add Exar Corporation to vendors list.

Signed-off-by: Nandor Han <nandor.han@ge.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index c7d1b72..ba9058c 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -104,6 +104,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
-- 
2.10.1

[PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Nandor Han]

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>
---
 drivers/gpio/Kconfig        |   5 +
 drivers/gpio/Makefile       |   1 +
 drivers/gpio/gpio-xra1403.c | 236 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 242 insertions(+)
 create mode 100644 drivers/gpio/gpio-xra1403.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 63ceed2..53cbe9b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1214,6 +1214,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 095598e..76dc3d7 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -140,6 +140,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..e6966d9
--- /dev/null
+++ b/drivers/gpio/gpio-xra1403.c
@@ -0,0 +1,236 @@
+/*
+ * 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>
+#include <linux/regmap.h>
+
+/* XRA1403 registers */
+#define XRA_GSR   0x00 /* GPIO State */
+#define XRA_OCR   0x02 /* Output Control */
+#define XRA_PIR   0x04 /* Input Polarity Inversion */
+#define XRA_GCR   0x06 /* GPIO Configuration */
+#define XRA_PUR   0x08 /* Input Internal Pull-up Resistor Enable/Disable */
+#define XRA_IER   0x0A /* Input Interrupt Enable */
+#define XRA_TSCR  0x0C /* Output Three-State Control */
+#define XRA_ISR   0x0E /* Input Interrupt Status */
+#define XRA_REIR  0x10 /* Input Rising Edge Interrupt Enable */
+#define XRA_FEIR  0x12 /* Input Falling Edge Interrupt Enable */
+#define XRA_IFR   0x14 /* Input Filter Enable/Disable */
+
+struct xra1403 {
+	struct gpio_chip  chip;
+	struct regmap     *regmap;
+};
+
+static const struct regmap_config xra1403_regmap_cfg = {
+		.reg_bits = 7,
+		.pad_bits = 1,
+		.val_bits = 8,
+
+		.max_register = XRA_IFR | 0x01,
+};
+
+static unsigned int to_reg(unsigned int reg, unsigned int offset)
+{
+	return reg + (offset > 7);
+}
+
+static int xra1403_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	struct xra1403 *xra = gpiochip_get_data(chip);
+
+	return regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+			BIT(offset % 8), BIT(offset % 8));
+}
+
+static int xra1403_direction_output(struct gpio_chip *chip, unsigned int offset,
+				    int value)
+{
+	int ret;
+	struct xra1403 *xra = gpiochip_get_data(chip);
+
+	ret = regmap_update_bits(xra->regmap, to_reg(XRA_GCR, offset),
+			BIT(offset % 8), 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(xra->regmap, to_reg(XRA_OCR, offset),
+			BIT(offset % 8), value ? BIT(offset % 8) : 0);
+
+	return ret;
+}
+
+static int xra1403_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	int ret;
+	unsigned int val;
+	struct xra1403 *xra = gpiochip_get_data(chip);
+
+	ret = regmap_read(xra->regmap, to_reg(XRA_GCR, offset), &val);
+	if (ret)
+		return ret;
+
+	return !!(val & BIT(offset % 8));
+}
+
+static int xra1403_get(struct gpio_chip *chip, unsigned int offset)
+{
+	int ret;
+	unsigned int val;
+	struct xra1403 *xra = gpiochip_get_data(chip);
+
+	ret = regmap_read(xra->regmap, to_reg(XRA_GSR, offset), &val);
+	if (ret)
+		return ret;
+
+	return !!(val & BIT(offset % 8));
+}
+
+static void xra1403_set(struct gpio_chip *chip, unsigned int offset, int value)
+{
+	int ret;
+	struct xra1403 *xra = gpiochip_get_data(chip);
+
+	ret = regmap_update_bits(xra->regmap, to_reg(XRA_OCR, offset),
+			BIT(offset % 8), value ? BIT(offset % 8) : 0);
+	if (ret)
+		dev_err(chip->parent, "Failed to set pin: %d, ret: %d\n",
+				offset, ret);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void xra1403_dbg_show(struct seq_file *s, struct gpio_chip *chip)
+{
+	int reg;
+	struct xra1403 *xra = gpiochip_get_data(chip);
+	int value[xra1403_regmap_cfg.max_register];
+	int i;
+	unsigned int gcr;
+	unsigned int gsr;
+
+	seq_puts(s, "xra reg:");
+	for (reg = 0; reg <= xra1403_regmap_cfg.max_register; reg++)
+		seq_printf(s, " %2.2x", reg);
+	seq_puts(s, "\n  value:");
+	for (reg = 0; reg < xra1403_regmap_cfg.max_register; reg++) {
+		regmap_read(xra->regmap, reg, &value[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;
+	int ret;
+
+	xra = devm_kzalloc(&spi->dev, sizeof(*xra), GFP_KERNEL);
+	if (!xra)
+		return -ENOMEM;
+
+	/* bring the chip out of reset if reset pin is provided*/
+	reset_gpio = devm_gpiod_get_optional(&spi->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(reset_gpio))
+		dev_warn(&spi->dev, "Could not get reset-gpios\n");
+
+	xra->chip.direction_input = xra1403_direction_input;
+	xra->chip.direction_output = xra1403_direction_output;
+	xra->chip.get_direction = xra1403_get_direction;
+	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->regmap = devm_regmap_init_spi(spi, &xra1403_regmap_cfg);
+	if (IS_ERR(xra->regmap)) {
+		ret = PTR_ERR(xra->regmap);
+		dev_err(&spi->dev, "Failed to allocate regmap: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_gpiochip_add_data(&spi->dev, &xra->chip, xra);
+	if (ret < 0) {
+		dev_err(&spi->dev, "Unable to register gpiochip\n");
+		return ret;
+	}
+
+	spi_set_drvdata(spi, xra);
+
+	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,
+	.id_table = xra1403_ids,
+	.driver   = {
+		.name           = "xra1403",
+		.of_match_table = of_match_ptr(xra1403_spi_of_match),
+	},
+};
+
+module_spi_driver(xra1403_driver);
+
+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

[PATCH v2 3/4] doc,dts - add XRA1403 DTS binding documentation

[Nandor Han]

Add the XRA1403 DTS binding documentation.

Signed-off-by: Nandor Han <nandor.han@ge.com>
---
 .../devicetree/bindings/gpio/gpio-xra1403.txt      | 46 ++++++++++++++++++++++
 1 file changed, 46 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..e13cc39
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-xra1403.txt
@@ -0,0 +1,46 @@
+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 - marks the node as gpio.
+	- #gpio-cells - should be two where the first cell is the pin number
+		and the second one is used for optional parameters.
+
+Optional properties:
+-------------------
+	- reset-gpios: in case available used to control the device reset line.
+	- interrupt-controller - marks the node as interrupt controller.
+	- #interrupt-cells - should be two and represents the number of cells
+		needed to encode interrupt source.
+
+Example
+--------
+
+	gpioxra0: gpio@2 {
+		compatible = "exar,xra1403";
+		reg = <2>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		reset-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+		spi-max-frequency = <1000000>;
+	};
-- 
2.10.1

[PATCH v2 4/4] Add XRA1403 support to MAINTAINERS file

[Nandor Han]

Add 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 892e958..0c5b984 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14013,6 +14013,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

Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]

Hi Nandor,

[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.11-rc6 next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nandor-Han/XRA1403-gpio-add-XRA1403-gpio-expander-driver/20170413-215739
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: frv-allyesconfig (attached as .config)
compiler: frv-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=frv 

All errors (new ones prefixed by >>):

   drivers//gpio/gpio-xra1403.c: In function 'xra1403_dbg_show':
>> drivers//gpio/gpio-xra1403.c:133:2: error: implicit declaration of function 'seq_puts' [-Werror=implicit-function-declaration]
     seq_puts(s, "xra reg:");
     ^~~~~~~~
>> drivers//gpio/gpio-xra1403.c:135:3: error: implicit declaration of function 'seq_printf' [-Werror=implicit-function-declaration]
      seq_printf(s, " %2.2x", reg);
      ^~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/seq_puts +133 drivers//gpio/gpio-xra1403.c

   127		struct xra1403 *xra = gpiochip_get_data(chip);
   128		int value[xra1403_regmap_cfg.max_register];
   129		int i;
   130		unsigned int gcr;
   131		unsigned int gsr;
   132	
 > 133		seq_puts(s, "xra reg:");
   134		for (reg = 0; reg <= xra1403_regmap_cfg.max_register; reg++)
 > 135			seq_printf(s, " %2.2x", reg);
   136		seq_puts(s, "\n  value:");
   137		for (reg = 0; reg < xra1403_regmap_cfg.max_register; reg++) {
   138			regmap_read(xra->regmap, reg, &value[reg]);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 47568 bytes --]

Re: [PATCH v2 1/4] dt-bindings: gpio - add exar to vendor prefixes list

[Rob Herring]

On Thu, Apr 13, 2017 at 01:27:48PM +0300, Nandor Han wrote:
> Add Exar Corporation to vendors list.

This patch has nothing to do with gpio, so that should be dropped from 
the subject.

> 
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +

Otherwise,

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

Re: [PATCH v2 3/4] doc,dts - add XRA1403 DTS binding documentation

[Rob Herring]

On Thu, Apr 13, 2017 at 01:27:50PM +0300, Nandor Han wrote:
> Add the XRA1403 DTS binding documentation.

"dt-bindings: gpio: ..." for the subject.

> 
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> ---
>  .../devicetree/bindings/gpio/gpio-xra1403.txt      | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-xra1403.txt

Otherwise,

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

Re: [PATCH v2 1/4] dt-bindings: gpio - add exar to vendor prefixes list

[Linus Walleij]

On Thu, Apr 13, 2017 at 12:27 PM, Nandor Han <nandor.han@ge.com> wrote:

> Add Exar Corporation to vendors list.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>

This patch applied to the GPIO tree with Rob's ACK and
the suggested subject change.

Yours,
Linus Walleij

Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Linus Walleij]

On Thu, Apr 13, 2017 at 12:27 PM, 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.
> 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>

I almost want to make the driver depend on !GPIO_SYSFS because
of this commit message.

DO NOT USE OR ENCOURAGE THE USE OF THE GPIO SYSFS
INTERFACE.

Use the character device.

Use the example in tools/gpio/* as a guideline and testbed.

Use libgpiod as a rich userspace.

And the commit message should state that this is a driver
for such and such Exar hardware instead.

Thanks.

> +#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>
> +#include <linux/regmap.h>

You are missing
#include <linux/seq_file.h>

and that is why the build robot is complaining.

Yours,
Linus Walleij

Re: [PATCH v2 3/4] doc,dts - add XRA1403 DTS binding documentation

[Linus Walleij]

On Thu, Apr 13, 2017 at 12:27 PM, Nandor Han <nandor.han@ge.com> wrote:

> Add the XRA1403 DTS binding documentation.
>
> Signed-off-by: Nandor Han <nandor.han@ge.com>

Patch applied with altered subject and Rob's ACK.

Yours,
Linus Walleij

[PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Han, Nandor (GE Healthcare)]

> -----Original Message-----
> From: Linus Walleij [mailto:linus.walleij@linaro.org]
> Sent: 24 April 2017 16:47
> To: Han, Nandor (GE Healthcare) <nandor.han@ge.com>
> Cc: Greg KH <gregkh@linuxfoundation.org>; David S. Miller <davem@davemloft.net>; Geert Uytterhoeven <geert@linux-
> m68k.org>; Mauro Carvalho Chehab <mchehab@kernel.org>; Daniel Vetter <daniel.vetter@ffwll.ch>; 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 v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver
> 
> On Thu, Apr 13, 2017 at 12:27 PM, 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.
> > 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>
> 
> I almost want to make the driver depend on !GPIO_SYSFS because
> of this commit message.
> 
> DO NOT USE OR ENCOURAGE THE USE OF THE GPIO SYSFS
> INTERFACE.
> 

Ahh.. I forgot to change this commit message. Sorry I will change it ASAP.

> Use the character device.
> 
> Use the example in tools/gpio/* as a guideline and testbed.
> 
> Use libgpiod as a rich userspace.
> 
> And the commit message should state that this is a driver
> for such and such Exar hardware instead.
> 
> Thanks.
> 
> > +#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>
> > +#include <linux/regmap.h>
> 
> You are missing
> #include <linux/seq_file.h>
> 
> and that is why the build robot is complaining.
> 

I've noticed that. I will change it in the next rev.

> Yours,
> Linus Walleij

Thanks,
   Nandy

Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Benjamin Henrion]

On Mon, Apr 24, 2017 at 3:47 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Apr 13, 2017 at 12:27 PM, 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.
>> 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>
>
> I almost want to make the driver depend on !GPIO_SYSFS because
> of this commit message.
>
> DO NOT USE OR ENCOURAGE THE USE OF THE GPIO SYSFS
> INTERFACE.
>
> Use the character device.

I doubt you will be able to convince the majority of people toggling
GPIOs via a simple shell script to switch to write a complex C
program. Not to mention cross compilation and the libraries
dependencies here.

Is there some good cli tools to access the new char device? If they
are shipped with most distros, that would reduce the pain.

Best,

--
Benjamin Henrion <bhenrion at ffii.org>
FFII Brussels - +32-484-566109 - +32-2-3500762
"In July 2005, after several failed attempts to legalise software
patents in Europe, the patent establishment changed its strategy.
Instead of explicitly seeking to sanction the patentability of
software, they are now seeking to create a central European patent
court, which would establish and enforce patentability rules in their
favor, without any possibility of correction by competing courts or
democratically elected legislators."

Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Geert Uytterhoeven]

Hi Benjamin,

On Tue, Apr 25, 2017 at 9:07 AM, Benjamin Henrion <zoobab@gmail.com> wrote:
> On Mon, Apr 24, 2017 at 3:47 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Thu, Apr 13, 2017 at 12:27 PM, 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.
>>> 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>
>>
>> I almost want to make the driver depend on !GPIO_SYSFS because
>> of this commit message.
>>
>> DO NOT USE OR ENCOURAGE THE USE OF THE GPIO SYSFS
>> INTERFACE.
>>
>> Use the character device.
>
> I doubt you will be able to convince the majority of people toggling
> GPIOs via a simple shell script to switch to write a complex C
> program. Not to mention cross compilation and the libraries
> dependencies here.
>
> Is there some good cli tools to access the new char device? If they
> are shipped with most distros, that would reduce the pain.

https://github.com/brgl/libgpiod

A bit early to expect it to be shipped with all distros, though.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Linus Walleij]

On Tue, Apr 25, 2017 at 9:07 AM, Benjamin Henrion <zoobab@gmail.com> wrote:

> I doubt you will be able to convince the majority of people toggling
> GPIOs via a simple shell script to switch to write a complex C
> program. Not to mention cross compilation and the libraries
> dependencies here.

I do not need to convince anyone, I'm not into politics.

The way to attract users to the character device is by offering better
features... so we smack in the following goodies:

- Need a userspace ABI? No more need to select CONFIG_GPIO_SYSFS!
  The chardev is always there for any gpio chip in newer kernels!
  Board vendors would have to actively delete core code to disable it!

- Need open drain? The chardev will support that, the sysfs will never.

- Need to set/get multiple lines with a single context switch? Chardev
  does this. Also the set operation will turn into a single register write
  if your driver implements .set_multiple()

- All future needs: line biasing? Schmitt triggers? Drive strengths?
  All that will use the character device, and the sysfs ABI will never
  support any of it.

> Is there some good cli tools to access the new char device? If they
> are shipped with most distros, that would reduce the pain.

I have those that come with the kernel:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/gpio

Then libgpiod as mentioned:
https://github.com/brgl/libgpiod/tree/master/src/tools

Yours,
Linus Walleij

Re: [PATCH v2 2/4] gpio - Add EXAR XRA1403 SPI GPIO expander driver

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 10:15 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

>> Is there some good cli tools to access the new char device? If they
>> are shipped with most distros, that would reduce the pain.
>
> https://github.com/brgl/libgpiod
>
> A bit early to expect it to be shipped with all distros, though.

Buildroot has it.
For kernel development it is quite enough.

-- 
With Best Regards,
Andy Shevchenko