[PATCH v7 0/3] gpio: add DT support for memory-mapped GPIOs

[PATCH v7 0/3] gpio: add DT support for memory-mapped GPIOs

[Christian Lamparter]

This patch series adds device tree support for generic memory-mapped GPIOs.
The GPIO library already allows drivers and architecture support code to
reuse generic code for managing a GPIO chip. Currently, a developer has
to create a platform device "basic-mmio-gpio" and attach a bgpio_pdata
platform data structure to make use of it. However, for architectures
which rely on the device tree to enumerate devices, creating custom
platform devices is another extra step that can be avoided by having
direct support via a device tree binding.

I initially came across this patch [0] from Álvaro Fernández Rojas,
while looking for an easy way to add support for the GPIO of my
WD MyBook Live [1] (APM82181). His generic approach patch allowed
me to easily get the GPIO (and the connected LEDs, buttons, gpiohogs)
up and running. Even thought, Mr. Fernandez initially developed it
for his work on the brcm63xx [2].

The drivers for gpio-clps711x, gpio-ge, gpio-moxart and gpio-ts4800
are now part of the gpio-mmio.c driver. The old driver files have
been removed and the Kconfig, Makefile entries have been updated
accordingly.

And finally, the most important stat about the series:
	>>> 333 insertions(+), 411 deletions(-) <<<
	It still removes more lines than it adds!

Thanks!

[0] <https://patchwork.ozlabs.org/patch/422121/>
[1] <https://github.com/chunkeey/MBL-openwrt>
[2] <https://wiki.openwrt.org/doc/hardware/soc/soc.broadcom.bcm63xx>

changelog:

v6 -> v7:
	- finally made a PATCH series (based on linux-gpio devel branch)
	- added Rob Herring's ACK to the dt bindings
	- cc'd clps711x, gpio-ge, moxart and ts4800 authors for driver
	  changes.

v5 -> v6:
	- rewrote bindings and driver patch to fit the wd,mbl-gpio
	- unified parser code for gpio-ge, gpio-moxart and gpio-ts4800
	- fixed gpio chip's base being overwritten with bogus "0"
	- fixed compat driver crash when reload gpio-generic.ko module
	- dropped already applied patches from the series
	- rebased code on linus' devel tree
	- moved dt bindings patch to the top of the series

v4 -> v5:
	- reverted rename of gpio-mmio.c back to gpio-generic.c
	- fixed Andy Shevchenko's comments
	- consolidated changes from clps711x, gpio-ge, gpio-moxart and
	  gpio-ts4800 into one patch.

v3 -> v4:
	- renamed gpio-generic.c to gpio-mmio.c
	- changed compat. string to "linux,gpio-mmio"
	- integrated Cirrus clps711x driver
	- integrated GE FGPA gpio-ge driver
	- integrated MOXA ART GPIO driver
	- integrated TS4800 gpio driver
	- reshuffled patches, reworded commits, fixed spelling errors, etc.

Christian Lamparter (2):
  gpio: dt-bindings: add wd,mbl-gpio bindings
  gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-mmio

Álvaro Fernández Rojas (1):
  gpio: mmio: add DT support for memory-mapped GPIOs

 .../devicetree/bindings/gpio/wd,mbl-gpio.txt       |  38 +++
 drivers/gpio/Kconfig                               |  43 +--
 drivers/gpio/Makefile                              |   4 -
 drivers/gpio/gpio-clps711x.c                       |  91 -------
 drivers/gpio/gpio-ge.c                             | 114 --------
 drivers/gpio/gpio-mmio.c                           | 289 ++++++++++++++++++++-
 drivers/gpio/gpio-moxart.c                         |  84 ------
 drivers/gpio/gpio-ts4800.c                         |  81 ------
 8 files changed, 333 insertions(+), 411 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
 delete mode 100644 drivers/gpio/gpio-clps711x.c
 delete mode 100644 drivers/gpio/gpio-ge.c
 delete mode 100644 drivers/gpio/gpio-moxart.c
 delete mode 100644 drivers/gpio/gpio-ts4800.c

-- 
2.8.1

[PATCH v7 1/3] gpio: dt-bindings: add wd,mbl-gpio bindings

[Christian Lamparter]

This patch adds the device tree bindings for the Western Digital's
MyBook Live memory-mapped GPIO controllers.

The gpios will be supported by gpio-mmio code of the
GPIO generic library.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 .../devicetree/bindings/gpio/wd,mbl-gpio.txt       | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt

diff --git a/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
new file mode 100644
index 0000000..038c3a6
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
@@ -0,0 +1,38 @@
+Bindings for the Western Digital's MyBook Live memory-mapped GPIO controllers.
+
+The Western Digital MyBook Live has two memory-mapped GPIO controllers.
+Both GPIO controller only have a single 8-bit data register, where GPIO
+state can be read and/or written.
+
+Required properties:
+	- compatible: should be "wd,mbl-gpio"
+	- reg-names: must contain
+		"dat" - data register
+	- reg: address + size pairs describing the GPIO register sets;
+		order must correspond with the order of entries in reg-names
+	- #gpio-cells: must be set to 2. The first cell is the pin number and
+			the second cell is used to specify the GPIO polarity:
+			0 = active high
+			1 = active low
+	- gpio-controller: marks the device node as a GPIO controller.
+
+Optional properties:
+	- no-output: GPIOs are read-only.
+
+Examples:
+	GPIO0: gpio0@e0000000 {
+		compatible = "wd,mbl-gpio";
+		reg-names = "dat";
+		reg = <0xe0000000 0x1>;
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+
+	GPIO1: gpio1@e0100000 {
+		compatible = "wd,mbl-gpio";
+		reg-names = "dat";
+		reg = <0xe0100000 0x1>;
+		#gpio-cells = <2>;
+		gpio-controller;
+		no-output;
+	};
-- 
2.8.1

[PATCH v7 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Christian Lamparter]

From: Álvaro Fernández Rojas <noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch adds support for defining memory-mapped GPIOs which
are compatible with the existing gpio-mmio interface. The generic
library provides support for many memory-mapped GPIO controllers
that are found in various on-board FPGA and ASIC solutions that
are used to control board's switches, LEDs, chip-selects,
Ethernet/USB PHY power, etc.

For setting GPIO's there are three configurations:
	1. single input/output register resource (named "dat"),
	2. set/clear pair (named "set" and "clr"),
	3. single output register resource and single input resource
	   ("set" and dat").

The configuration is detected by which resources are present.
For the single output register, this drives a 1 by setting a bit
and a zero by clearing a bit.  For the set clr pair, this drives
a 1 by setting a bit in the set register and clears it by setting
a bit in the clear register. The configuration is detected by
which resources are present.

For setting the GPIO direction, there are three configurations:
	a. simple bidirectional GPIOs that requires no configuration.
	b. an output direction register (named "dirout")
	   where a 1 bit indicates the GPIO is an output.
	c. an input direction register (named "dirin")
	   where a 1 bit indicates the GPIO is an input.

The first user for this binding is "wd,mbl-gpio".

Signed-off-by: Álvaro Fernández Rojas <noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 drivers/gpio/gpio-mmio.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 6c1cb3b..1cfb70a 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -61,6 +61,8 @@ o        `                     ~~~~\___/~~~~    ` controller in FPGA is ,.`
 #include <linux/bitops.h>
 #include <linux/platform_device.h>
 #include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 static void bgpio_write8(void __iomem *reg, unsigned long data)
 {
@@ -569,6 +571,89 @@ static void __iomem *bgpio_map(struct platform_device *pdev,
 	return devm_ioremap_resource(&pdev->dev, r);
 }
 
+#ifdef CONFIG_OF
+static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
+				     struct bgpio_pdata *pdata,
+				     unsigned long *flags)
+{
+	struct device *dev = &pdev->dev;
+	int err;
+
+	pdata->base = -1;
+	/* If ngpio property is not specified, of_property_read_u32
+	 * will return -EINVAL. In this case the number of GPIOs is
+	 * automatically determined by the register width. Any
+	 * other error of of_property_read_u32 is due bad data and
+	 * needs to be dealt with.
+	 */
+	err = of_property_read_u32(dev->of_node, "ngpio", &pdata->ngpio);
+	if (err && err != -EINVAL)
+		return err;
+
+	if (of_device_is_big_endian(dev->of_node))
+		*flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+
+	if (of_property_read_bool(dev->of_node, "unreadable-reg-set"))
+		*flags |= BGPIOF_UNREADABLE_REG_SET;
+
+	if (of_property_read_bool(dev->of_node, "unreadable-reg-dir"))
+		*flags |= BGPIOF_UNREADABLE_REG_DIR;
+
+	if (of_property_read_bool(dev->of_node, "big-endian-byte-order"))
+		*flags |= BGPIOF_BIG_ENDIAN;
+
+	if (of_property_read_bool(dev->of_node, "read-output-reg-set"))
+		*flags |= BGPIOF_READ_OUTPUT_REG_SET;
+
+	if (of_property_read_bool(dev->of_node, "no-output"))
+		*flags |= BGPIOF_NO_OUTPUT;
+	return 0;
+}
+
+#define ADD_GPIO_OF(_name, _func) { .compatible = _name, .data = _func }
+
+static const struct of_device_id bgpio_of_match[] = {
+	ADD_GPIO_OF("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
+	{ }
+};
+#undef ADD_GPIO_OF
+MODULE_DEVICE_TABLE(of, bgpio_of_match);
+
+static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
+					  unsigned long *flags)
+{
+	const int (*parse_dt)(struct platform_device *,
+			      struct bgpio_pdata *, unsigned long *);
+	const struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *of_id;
+	struct bgpio_pdata *pdata;
+	int err = -ENODEV;
+
+	of_id = of_match_node(bgpio_of_match, node);
+	if (!of_id)
+		return NULL;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(struct bgpio_pdata),
+			     GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	parse_dt = (const void *)of_id->data;
+	if (parse_dt)
+		err = parse_dt(pdev, pdata, flags);
+	if (err)
+		return ERR_PTR(err);
+
+	return pdata;
+}
+#else
+static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
+					  unsigned long *flags)
+{
+	return NULL;
+}
+#endif /* CONFIG_OF */
+
 static int bgpio_pdev_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -579,10 +664,19 @@ static int bgpio_pdev_probe(struct platform_device *pdev)
 	void __iomem *dirout;
 	void __iomem *dirin;
 	unsigned long sz;
-	unsigned long flags = pdev->id_entry->driver_data;
+	unsigned long flags = 0;
 	int err;
 	struct gpio_chip *gc;
-	struct bgpio_pdata *pdata = dev_get_platdata(dev);
+	struct bgpio_pdata *pdata;
+
+	pdata = bgpio_parse_dt(pdev, &flags);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+
+	if (!pdata) {
+		pdata = dev_get_platdata(dev);
+		flags = pdev->id_entry->driver_data;
+	}
 
 	r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dat");
 	if (!r)
@@ -646,6 +740,7 @@ MODULE_DEVICE_TABLE(platform, bgpio_id_table);
 static struct platform_driver bgpio_driver = {
 	.driver = {
 		.name = "basic-mmio-gpio",
+		.of_match_table = of_match_ptr(bgpio_of_match),
 	},
 	.id_table = bgpio_id_table,
 	.probe = bgpio_pdev_probe,
-- 
2.8.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

[PATCH v7 3/3] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-mmio

[Christian Lamparter]

This patch integrates these GPIO drivers into gpio-mmio.

Cc: Alexander Shiyan <shc_work@mail.ru>
Cc: Julien Grossholtz <julien.grossholtz@savoirfairelinux.com>
Cc: Martyn Welch <martyn.welch@ge.com>
Cc: Jonas Jensen <jonas.jensen@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/Kconfig         |  43 ++--------
 drivers/gpio/Makefile        |   4 -
 drivers/gpio/gpio-clps711x.c |  91 ---------------------
 drivers/gpio/gpio-ge.c       | 114 --------------------------
 drivers/gpio/gpio-mmio.c     | 190 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpio-moxart.c   |  84 -------------------
 drivers/gpio/gpio-ts4800.c   |  81 ------------------
 7 files changed, 198 insertions(+), 409 deletions(-)
 delete mode 100644 drivers/gpio/gpio-clps711x.c
 delete mode 100644 drivers/gpio/gpio-ge.c
 delete mode 100644 drivers/gpio/gpio-moxart.c
 delete mode 100644 drivers/gpio/gpio-ts4800.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index a68d838..d3c0d00 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -151,13 +151,6 @@ config GPIO_BRCMSTB
 	help
 	  Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
 
-config GPIO_CLPS711X
-	tristate "CLPS711X GPIO support"
-	depends on ARCH_CLPS711X || COMPILE_TEST
-	select GPIO_GENERIC
-	help
-	  Say yes here to support GPIO on CLPS711X SoCs.
-
 config GPIO_DAVINCI
 	bool "TI Davinci/Keystone GPIO support"
 	default y if ARCH_DAVINCI
@@ -193,22 +186,18 @@ config GPIO_ETRAXFS
 	help
 	  Say yes here to support the GPIO controller on Axis ETRAX FS SoCs.
 
-config GPIO_GE_FPGA
-	bool "GE FPGA based GPIO"
-	depends on GE_FPGA
-	select GPIO_GENERIC
-	help
-	  Support for common GPIO functionality provided on some GE Single Board
-	  Computers.
-
-	  This driver provides basic support (configure as input or output, read
-	  and write pin state) for GPIO implemented in a number of GE single
-	  board computers.
-
 config GPIO_GENERIC_PLATFORM
 	tristate "Generic memory-mapped GPIO controller support (MMIO platform device)"
 	select GPIO_GENERIC
 	help
+	  Select this to support many generic memory-mapped GPIO controllers.
+
+	  This driver also includes support for the following GPIOs:
+	    CLPS711X SoCs
+	    MOXA ART SoC
+	    TS-4800 FPGA DIO blocks and compatibles.
+	    GPIOs found on some GE Single Board Computers.
+
 	  Say yes here to support basic platform_device memory-mapped GPIO controllers.
 
 config GPIO_GRGPIO
@@ -285,14 +274,6 @@ config GPIO_MM_LANTIQ
 	  (EBU) found on Lantiq SoCs. The gpios are output only as they are
 	  created by attaching a 16bit latch to the bus.
 
-config GPIO_MOXART
-	bool "MOXART GPIO support"
-	depends on ARCH_MOXART || COMPILE_TEST
-	select GPIO_GENERIC
-	help
-	  Select this option to enable GPIO driver for
-	  MOXA ART SoC devices.
-
 config GPIO_MPC5200
 	def_bool y
 	depends on PPC_MPC52xx
@@ -404,14 +385,6 @@ config GPIO_TEGRA
 	default y
 	depends on ARCH_TEGRA || COMPILE_TEST
 
-config GPIO_TS4800
-	tristate "TS-4800 DIO blocks and compatibles"
-	depends on OF_GPIO
-	depends on SOC_IMX51 || COMPILE_TEST
-	select GPIO_GENERIC
-	help
-	  This driver support TS-4800 FPGA GPIO controllers.
-
 config GPIO_TZ1090
 	bool "Toumaz Xenif TZ1090 GPIO support"
 	depends on SOC_TZ1090
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 991598e..d8d63ae 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -31,7 +31,6 @@ obj-$(CONFIG_GPIO_ATH79)	+= gpio-ath79.o
 obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BRCMSTB)	+= gpio-brcmstb.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
-obj-$(CONFIG_GPIO_CLPS711X)	+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
 obj-$(CONFIG_GPIO_CRYSTAL_COVE)	+= gpio-crystalcove.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
@@ -43,7 +42,6 @@ obj-$(CONFIG_GPIO_EM)		+= gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_ETRAXFS)	+= gpio-etraxfs.o
 obj-$(CONFIG_GPIO_F7188X)	+= gpio-f7188x.o
-obj-$(CONFIG_GPIO_GE_FPGA)	+= gpio-ge.o
 obj-$(CONFIG_GPIO_GRGPIO)	+= gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH)		+= gpio-ich.o
 obj-$(CONFIG_GPIO_IOP)		+= gpio-iop.o
@@ -68,7 +66,6 @@ obj-$(CONFIG_GPIO_MC9S08DZ60)	+= gpio-mc9s08dz60.o
 obj-$(CONFIG_GPIO_MCP23S08)	+= gpio-mcp23s08.o
 obj-$(CONFIG_GPIO_ML_IOH)	+= gpio-ml-ioh.o
 obj-$(CONFIG_GPIO_MM_LANTIQ)	+= gpio-mm-lantiq.o
-obj-$(CONFIG_GPIO_MOXART)	+= gpio-moxart.o
 obj-$(CONFIG_GPIO_MPC5200)	+= gpio-mpc5200.o
 obj-$(CONFIG_GPIO_MPC8XXX)	+= gpio-mpc8xxx.o
 obj-$(CONFIG_GPIO_MSIC)		+= gpio-msic.o
@@ -107,7 +104,6 @@ obj-$(CONFIG_GPIO_TPS65218)	+= gpio-tps65218.o
 obj-$(CONFIG_GPIO_TPS6586X)	+= gpio-tps6586x.o
 obj-$(CONFIG_GPIO_TPS65910)	+= gpio-tps65910.o
 obj-$(CONFIG_GPIO_TPS65912)	+= gpio-tps65912.o
-obj-$(CONFIG_GPIO_TS4800)	+= gpio-ts4800.o
 obj-$(CONFIG_GPIO_TS5500)	+= gpio-ts5500.o
 obj-$(CONFIG_GPIO_TWL4030)	+= gpio-twl4030.o
 obj-$(CONFIG_GPIO_TWL6040)	+= gpio-twl6040.o
diff --git a/drivers/gpio/gpio-clps711x.c b/drivers/gpio/gpio-clps711x.c
deleted file mode 100644
index 5a69025..0000000
--- a/drivers/gpio/gpio-clps711x.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- *  CLPS711X GPIO driver
- *
- *  Copyright (C) 2012,2013 Alexander Shiyan <shc_work@mail.ru>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <linux/err.h>
-#include <linux/module.h>
-#include <linux/gpio/driver.h>
-#include <linux/platform_device.h>
-
-static int clps711x_gpio_probe(struct platform_device *pdev)
-{
-	struct device_node *np = pdev->dev.of_node;
-	void __iomem *dat, *dir;
-	struct gpio_chip *gc;
-	struct resource *res;
-	int err, id = np ? of_alias_get_id(np, "gpio") : pdev->id;
-
-	if ((id < 0) || (id > 4))
-		return -ENODEV;
-
-	gc = devm_kzalloc(&pdev->dev, sizeof(*gc), GFP_KERNEL);
-	if (!gc)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	dat = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(dat))
-		return PTR_ERR(dat);
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-	dir = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(dir))
-		return PTR_ERR(dir);
-
-	switch (id) {
-	case 3:
-		/* PORTD is inverted logic for direction register */
-		err = bgpio_init(gc, &pdev->dev, 1, dat, NULL, NULL,
-				 NULL, dir, 0);
-		break;
-	default:
-		err = bgpio_init(gc, &pdev->dev, 1, dat, NULL, NULL,
-				 dir, NULL, 0);
-		break;
-	}
-
-	if (err)
-		return err;
-
-	switch (id) {
-	case 4:
-		/* PORTE is 3 lines only */
-		gc->ngpio = 3;
-		break;
-	default:
-		break;
-	}
-
-	gc->base = id * 8;
-	gc->owner = THIS_MODULE;
-	platform_set_drvdata(pdev, gc);
-
-	return devm_gpiochip_add_data(&pdev->dev, gc, NULL);
-}
-
-static const struct of_device_id __maybe_unused clps711x_gpio_ids[] = {
-	{ .compatible = "cirrus,clps711x-gpio" },
-	{ }
-};
-MODULE_DEVICE_TABLE(of, clps711x_gpio_ids);
-
-static struct platform_driver clps711x_gpio_driver = {
-	.driver	= {
-		.name		= "clps711x-gpio",
-		.of_match_table	= of_match_ptr(clps711x_gpio_ids),
-	},
-	.probe	= clps711x_gpio_probe,
-};
-module_platform_driver(clps711x_gpio_driver);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Alexander Shiyan <shc_work@mail.ru>");
-MODULE_DESCRIPTION("CLPS711X GPIO driver");
-MODULE_ALIAS("platform:clps711x-gpio");
diff --git a/drivers/gpio/gpio-ge.c b/drivers/gpio/gpio-ge.c
deleted file mode 100644
index 8650b29..0000000
--- a/drivers/gpio/gpio-ge.c
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * Driver for GE FPGA based GPIO
- *
- * Author: Martyn Welch <martyn.welch@ge.com>
- *
- * 2008 (c) GE Intelligent Platforms Embedded Systems, Inc.
- *
- * This file is licensed under the terms of the GNU General Public License
- * version 2.  This program is licensed "as is" without any warranty of any
- * kind, whether express or implied.
- */
-
-/* TODO
- *
- * Configuration of output modes (totem-pole/open-drain)
- * Interrupt configuration - interrupts are always generated the FPGA relies on
- * the I/O interrupt controllers mask to stop them propergating
- */
-
-#include <linux/kernel.h>
-#include <linux/io.h>
-#include <linux/slab.h>
-#include <linux/of_device.h>
-#include <linux/of_gpio.h>
-#include <linux/of_address.h>
-#include <linux/module.h>
-#include <linux/gpio/driver.h>
-
-#define GEF_GPIO_DIRECT		0x00
-#define GEF_GPIO_IN		0x04
-#define GEF_GPIO_OUT		0x08
-#define GEF_GPIO_TRIG		0x0C
-#define GEF_GPIO_POLAR_A	0x10
-#define GEF_GPIO_POLAR_B	0x14
-#define GEF_GPIO_INT_STAT	0x18
-#define GEF_GPIO_OVERRUN	0x1C
-#define GEF_GPIO_MODE		0x20
-
-static const struct of_device_id gef_gpio_ids[] = {
-	{
-		.compatible	= "gef,sbc610-gpio",
-		.data		= (void *)19,
-	}, {
-		.compatible	= "gef,sbc310-gpio",
-		.data		= (void *)6,
-	}, {
-		.compatible	= "ge,imp3a-gpio",
-		.data		= (void *)16,
-	},
-	{ }
-};
-MODULE_DEVICE_TABLE(of, gef_gpio_ids);
-
-static int __init gef_gpio_probe(struct platform_device *pdev)
-{
-	const struct of_device_id *of_id =
-		of_match_device(gef_gpio_ids, &pdev->dev);
-	struct gpio_chip *gc;
-	void __iomem *regs;
-	int ret;
-
-	gc = devm_kzalloc(&pdev->dev, sizeof(*gc), GFP_KERNEL);
-	if (!gc)
-		return -ENOMEM;
-
-	regs = of_iomap(pdev->dev.of_node, 0);
-	if (!regs)
-		return -ENOMEM;
-
-	ret = bgpio_init(gc, &pdev->dev, 4, regs + GEF_GPIO_IN,
-			 regs + GEF_GPIO_OUT, NULL, NULL,
-			 regs + GEF_GPIO_DIRECT, BGPIOF_BIG_ENDIAN_BYTE_ORDER);
-	if (ret) {
-		dev_err(&pdev->dev, "bgpio_init failed\n");
-		goto err0;
-	}
-
-	/* Setup pointers to chip functions */
-	gc->label = devm_kstrdup(&pdev->dev, pdev->dev.of_node->full_name,
-				     GFP_KERNEL);
-	if (!gc->label) {
-		ret = -ENOMEM;
-		goto err0;
-	}
-
-	gc->base = -1;
-	gc->ngpio = (u16)(uintptr_t)of_id->data;
-	gc->of_gpio_n_cells = 2;
-	gc->of_node = pdev->dev.of_node;
-
-	/* This function adds a memory mapped GPIO chip */
-	ret = devm_gpiochip_add_data(&pdev->dev, gc, NULL);
-	if (ret)
-		goto err0;
-
-	return 0;
-err0:
-	iounmap(regs);
-	pr_err("%s: GPIO chip registration failed\n",
-			pdev->dev.of_node->full_name);
-	return ret;
-};
-
-static struct platform_driver gef_gpio_driver = {
-	.driver = {
-		.name		= "gef-gpio",
-		.of_match_table	= gef_gpio_ids,
-	},
-};
-module_platform_driver_probe(gef_gpio_driver, gef_gpio_probe);
-
-MODULE_DESCRIPTION("GE I/O FPGA GPIO driver");
-MODULE_AUTHOR("Martyn Welch <martyn.welch@ge.com");
-MODULE_LICENSE("GPL");
diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 1cfb70a..f116786 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -610,10 +610,200 @@ static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
 	return 0;
 }
 
+static int clps711x_parse_dt(struct platform_device *pdev,
+			     struct bgpio_pdata *pdata,
+			     unsigned long *flags)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res;
+	const char *dir_reg_name;
+	int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
+
+	if ((id < 0) || (id > 4))
+		return -ENODEV;
+
+	/* PORTE is 3 lines only */
+	pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0;
+
+	/* PORTD is inverted logic for direction register */
+	dir_reg_name = (id == 3) ? "dirin" : "dirout",
+
+	pdata->base = id * 8;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+	if (!res->name || strcmp("dat", res->name))
+		res->name = devm_kstrdup(&pdev->dev, "dat", GFP_KERNEL);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -EINVAL;
+	if (!res->name || strcmp(dir_reg_name, res->name))
+		res->name = devm_kstrdup(&pdev->dev, dir_reg_name, GFP_KERNEL);
+
+	return 0;
+}
+
+static int ge_dt_cb(struct platform_device *pdev,
+		    struct bgpio_pdata *pdata,
+		    unsigned long *flags)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	pdata->label = devm_kstrdup(&pdev->dev, np->full_name, GFP_KERNEL);
+	if (!pdata->label)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int moxart_dt_cb(struct platform_device *pdev,
+			struct bgpio_pdata *pdata,
+			unsigned long *flags)
+{
+	pdata->base = 0;
+	pdata->label = "moxart-gpio";
+	return 0;
+}
+
+
+static int ts4800_dt_cb(struct platform_device *pdev,
+			struct bgpio_pdata *pdata,
+			unsigned long *flags)
+{
+	int err;
+
+	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &pdata->ngpio);
+	if (err == -EINVAL) {
+		pdata->ngpio = 16;
+		err = 0;
+	}
+	return err;
+}
+
+struct compat_gpio_device_data {
+	unsigned int expected_resource_size;
+	unsigned int ngpio;
+	resource_size_t register_width;
+	unsigned long flags;
+	int (*call_back)(struct platform_device *pdev,
+			 struct bgpio_pdata *pdata,
+			 unsigned long *flags);
+	struct resource_replacement {
+		resource_size_t start_offset;
+		const char *name;
+	} resources[5];
+};
+
+#define ADD_COMPAT_REGISTER(_name, _offset)	\
+	{ .name = (_name), .start_offset = (_offset) }
+
+#define ADD_COMPAT_GPIO(_comp, _sz, _ngpio, _width, _cb, _f, _res...)	\
+	{ .compatible = (_comp),					\
+	  .data = &(struct compat_gpio_device_data) {			\
+		.expected_resource_size = (_sz),			\
+		.ngpio = (_ngpio),					\
+		.register_width = (_width),				\
+		.flags = (_f),						\
+		.call_back = (_cb),					\
+		.resources = { _res },					\
+	}								\
+}
+
+#define ADD_COMPAT_GE_GPIO(_name, _ngpio)				\
+	ADD_COMPAT_GPIO(_name, 0x24, _ngpio, 0x4, ge_dt_cb,		\
+		 BGPIOF_BIG_ENDIAN_BYTE_ORDER,				\
+		 ADD_COMPAT_REGISTER("dat", 0x04),			\
+		 ADD_COMPAT_REGISTER("set", 0x08),			\
+		 ADD_COMPAT_REGISTER("dirin", 0x00))			\
+
+static const struct of_device_id compat_gpio_devices[] = {
+	ADD_COMPAT_GE_GPIO("ge,imp3a-gpio", 16),
+	ADD_COMPAT_GE_GPIO("gef,sbc310-gpio", 6),
+	ADD_COMPAT_GE_GPIO("gef,sbc610-gpio", 19),
+	ADD_COMPAT_GPIO("moxa,moxart-gpio", 0xc, 0, 0x4, moxart_dt_cb,
+		 BGPIOF_READ_OUTPUT_REG_SET,
+		 ADD_COMPAT_REGISTER("dat", 0x04),
+		 ADD_COMPAT_REGISTER("set", 0x00),
+		 ADD_COMPAT_REGISTER("dirout", 0x08)),
+	ADD_COMPAT_GPIO("technologic,ts4800-gpio", 0x6, 16, 0x2, ts4800_dt_cb,
+		 0, ADD_COMPAT_REGISTER("dat", 0x00),
+		 ADD_COMPAT_REGISTER("set", 0x02),
+		 ADD_COMPAT_REGISTER("dirout", 0x04)),
+};
+
+#undef ADD_COMPAT_GE_GPIO
+#undef ADD_COMPAT_GPIO
+#undef ADD_COMPAT_REGISTER
+
+static int compat_parse_dt(struct platform_device *pdev,
+			   struct bgpio_pdata *pdata,
+			   unsigned long *flags)
+{
+	const struct device_node *node = pdev->dev.of_node;
+	const struct compat_gpio_device_data *entry;
+	const struct of_device_id *of_id;
+	struct resource *res;
+	int err;
+
+	of_id = of_match_node(compat_gpio_devices, node);
+	if (!of_id)
+		return -ENODEV;
+
+	entry = of_id->data;
+	if (!entry || !entry->resources[0].name)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	if (!res->name || strcmp(entry->resources[0].name, res->name)) {
+		struct resource nres[ARRAY_SIZE(entry->resources)];
+		int i;
+
+		if (resource_size(res) != entry->expected_resource_size)
+			return -EINVAL;
+
+		for (i = 0; i < ARRAY_SIZE(entry->resources); i++) {
+			if (!entry->resources[i].name)
+				continue;
+
+			nres[i].name = devm_kstrdup(&pdev->dev,
+				entry->resources[i].name, GFP_KERNEL);
+			nres[i].start = res->start +
+				entry->resources[i].start_offset;
+			nres[i].end = nres[i].start +
+				entry->register_width - 1;
+			nres[i].flags = IORESOURCE_MEM;
+		}
+
+		err = platform_device_add_resources(pdev, nres, i);
+		if (err)
+			return err;
+	}
+
+	pdata->base = -1;
+	pdata->ngpio = entry->ngpio;
+	*flags = entry->flags;
+
+	if (entry->call_back)
+		err = entry->call_back(pdev, pdata, flags);
+
+	return err;
+}
+
 #define ADD_GPIO_OF(_name, _func) { .compatible = _name, .data = _func }
 
 static const struct of_device_id bgpio_of_match[] = {
 	ADD_GPIO_OF("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
+	ADD_GPIO_OF("cirrus,clps711x-gpio", clps711x_parse_dt),
+	ADD_GPIO_OF("ge,imp3a-gpio", compat_parse_dt),
+	ADD_GPIO_OF("gef,sbc310-gpio", compat_parse_dt),
+	ADD_GPIO_OF("gef,sbc610-gpio", compat_parse_dt),
+	ADD_GPIO_OF("moxa,moxart-gpio", compat_parse_dt),
+	ADD_GPIO_OF("technologic,ts4800-gpio", compat_parse_dt),
 	{ }
 };
 #undef ADD_GPIO_OF
diff --git a/drivers/gpio/gpio-moxart.c b/drivers/gpio/gpio-moxart.c
deleted file mode 100644
index d58d389..0000000
--- a/drivers/gpio/gpio-moxart.c
+++ /dev/null
@@ -1,84 +0,0 @@
-/*
- * MOXA ART SoCs GPIO driver.
- *
- * Copyright (C) 2013 Jonas Jensen
- *
- * Jonas Jensen <jonas.jensen@gmail.com>
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2.  This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/err.h>
-#include <linux/init.h>
-#include <linux/irq.h>
-#include <linux/io.h>
-#include <linux/platform_device.h>
-#include <linux/of_address.h>
-#include <linux/of_gpio.h>
-#include <linux/pinctrl/consumer.h>
-#include <linux/delay.h>
-#include <linux/timer.h>
-#include <linux/bitops.h>
-#include <linux/gpio/driver.h>
-
-#define GPIO_DATA_OUT		0x00
-#define GPIO_DATA_IN		0x04
-#define GPIO_PIN_DIRECTION	0x08
-
-static int moxart_gpio_probe(struct platform_device *pdev)
-{
-	struct device *dev = &pdev->dev;
-	struct resource *res;
-	struct gpio_chip *gc;
-	void __iomem *base;
-	int ret;
-
-	gc = devm_kzalloc(dev, sizeof(*gc), GFP_KERNEL);
-	if (!gc)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(base))
-		return PTR_ERR(base);
-
-	ret = bgpio_init(gc, dev, 4, base + GPIO_DATA_IN,
-			 base + GPIO_DATA_OUT, NULL,
-			 base + GPIO_PIN_DIRECTION, NULL,
-			 BGPIOF_READ_OUTPUT_REG_SET);
-	if (ret) {
-		dev_err(&pdev->dev, "bgpio_init failed\n");
-		return ret;
-	}
-
-	gc->label = "moxart-gpio";
-	gc->request = gpiochip_generic_request;
-	gc->free = gpiochip_generic_free;
-	gc->base = 0;
-	gc->owner = THIS_MODULE;
-
-	ret = devm_gpiochip_add_data(dev, gc, NULL);
-	if (ret) {
-		dev_err(dev, "%s: gpiochip_add failed\n",
-			dev->of_node->full_name);
-		return ret;
-	}
-
-	return ret;
-}
-
-static const struct of_device_id moxart_gpio_match[] = {
-	{ .compatible = "moxa,moxart-gpio" },
-	{ }
-};
-
-static struct platform_driver moxart_gpio_driver = {
-	.driver	= {
-		.name		= "moxart-gpio",
-		.of_match_table	= moxart_gpio_match,
-	},
-	.probe	= moxart_gpio_probe,
-};
-builtin_platform_driver(moxart_gpio_driver);
diff --git a/drivers/gpio/gpio-ts4800.c b/drivers/gpio/gpio-ts4800.c
deleted file mode 100644
index 0c144a7..0000000
--- a/drivers/gpio/gpio-ts4800.c
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- * GPIO driver for the TS-4800 board
- *
- * Copyright (c) 2016 - Savoir-faire Linux
- *
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#include <linux/gpio/driver.h>
-#include <linux/of_address.h>
-#include <linux/of_device.h>
-#include <linux/platform_device.h>
-
-#define DEFAULT_PIN_NUMBER      16
-#define INPUT_REG_OFFSET        0x00
-#define OUTPUT_REG_OFFSET       0x02
-#define DIRECTION_REG_OFFSET    0x04
-
-static int ts4800_gpio_probe(struct platform_device *pdev)
-{
-	struct device_node *node;
-	struct gpio_chip *chip;
-	struct resource *res;
-	void __iomem *base_addr;
-	int retval;
-	u32 ngpios;
-
-	chip = devm_kzalloc(&pdev->dev, sizeof(struct gpio_chip), GFP_KERNEL);
-	if (!chip)
-		return -ENOMEM;
-
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	base_addr = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(base_addr))
-		return PTR_ERR(base_addr);
-
-	node = pdev->dev.of_node;
-	if (!node)
-		return -EINVAL;
-
-	retval = of_property_read_u32(node, "ngpios", &ngpios);
-	if (retval == -EINVAL)
-		ngpios = DEFAULT_PIN_NUMBER;
-	else if (retval)
-		return retval;
-
-	retval = bgpio_init(chip, &pdev->dev, 2, base_addr + INPUT_REG_OFFSET,
-			    base_addr + OUTPUT_REG_OFFSET, NULL,
-			    base_addr + DIRECTION_REG_OFFSET, NULL, 0);
-	if (retval) {
-		dev_err(&pdev->dev, "bgpio_init failed\n");
-		return retval;
-	}
-
-	chip->ngpio = ngpios;
-
-	platform_set_drvdata(pdev, chip);
-
-	return devm_gpiochip_add_data(&pdev->dev, chip, NULL);
-}
-
-static const struct of_device_id ts4800_gpio_of_match[] = {
-	{ .compatible = "technologic,ts4800-gpio", },
-	{},
-};
-
-static struct platform_driver ts4800_gpio_driver = {
-	.driver = {
-		   .name = "ts4800-gpio",
-		   .of_match_table = ts4800_gpio_of_match,
-		   },
-	.probe = ts4800_gpio_probe,
-};
-
-module_platform_driver_probe(ts4800_gpio_driver, ts4800_gpio_probe);
-
-MODULE_AUTHOR("Julien Grossholtz <julien.grossholtz@savoirfairelinux.com>");
-MODULE_DESCRIPTION("TS4800 FPGA GPIO driver");
-MODULE_LICENSE("GPL v2");
-- 
2.8.1

Re: [PATCH v7 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Andy Shevchenko]

On Fri, May 6, 2016 at 2:10 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> From: Álvaro Fernández Rojas <noltari@gmail.com>
>
> This patch adds support for defining memory-mapped GPIOs which
> are compatible with the existing gpio-mmio interface. The generic
> library provides support for many memory-mapped GPIO controllers
> that are found in various on-board FPGA and ASIC solutions that
> are used to control board's switches, LEDs, chip-selects,
> Ethernet/USB PHY power, etc.
>
> For setting GPIO's there are three configurations:
>         1. single input/output register resource (named "dat"),
>         2. set/clear pair (named "set" and "clr"),
>         3. single output register resource and single input resource
>            ("set" and dat").
>
> The configuration is detected by which resources are present.
> For the single output register, this drives a 1 by setting a bit
> and a zero by clearing a bit.  For the set clr pair, this drives
> a 1 by setting a bit in the set register and clears it by setting
> a bit in the clear register. The configuration is detected by
> which resources are present.
>
> For setting the GPIO direction, there are three configurations:
>         a. simple bidirectional GPIOs that requires no configuration.
>         b. an output direction register (named "dirout")
>            where a 1 bit indicates the GPIO is an output.
>         c. an input direction register (named "dirin")
>            where a 1 bit indicates the GPIO is an input.
>
> The first user for this binding is "wd,mbl-gpio".

Some (mostly) style issues below.

> @@ -569,6 +571,89 @@ static void __iomem *bgpio_map(struct platform_device *pdev,
>         return devm_ioremap_resource(&pdev->dev, r);
>  }
>
> +#ifdef CONFIG_OF
> +static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
> +                                    struct bgpio_pdata *pdata,
> +                                    unsigned long *flags)
> +{
> +       struct device *dev = &pdev->dev;
> +       int err;
> +
> +       pdata->base = -1;

+ empty line

> +       /* If ngpio property is not specified, of_property_read_u32
> +        * will return -EINVAL. In this case the number of GPIOs is
> +        * automatically determined by the register width. Any
> +        * other error of of_property_read_u32 is due bad data and
> +        * needs to be dealt with.

Couple style issues:
/*
 * First sentence starts here. func() are going with parens.
 */

> +        */
> +       err = of_property_read_u32(dev->of_node, "ngpio", &pdata->ngpio);
> +       if (err && err != -EINVAL)
> +               return err;
> +
> +       if (of_device_is_big_endian(dev->of_node))
> +               *flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
> +
> +       if (of_property_read_bool(dev->of_node, "unreadable-reg-set"))
> +               *flags |= BGPIOF_UNREADABLE_REG_SET;
> +
> +       if (of_property_read_bool(dev->of_node, "unreadable-reg-dir"))
> +               *flags |= BGPIOF_UNREADABLE_REG_DIR;
> +
> +       if (of_property_read_bool(dev->of_node, "big-endian-byte-order"))
> +               *flags |= BGPIOF_BIG_ENDIAN;
> +
> +       if (of_property_read_bool(dev->of_node, "read-output-reg-set"))
> +               *flags |= BGPIOF_READ_OUTPUT_REG_SET;
> +
> +       if (of_property_read_bool(dev->of_node, "no-output"))
> +               *flags |= BGPIOF_NO_OUTPUT;
> +       return 0;
> +}
> +
> +#define ADD_GPIO_OF(_name, _func) { .compatible = _name, .data = _func }
> +
> +static const struct of_device_id bgpio_of_match[] = {
> +       ADD_GPIO_OF("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
> +       { }
> +};
> +#undef ADD_GPIO_OF
> +MODULE_DEVICE_TABLE(of, bgpio_of_match);
> +
> +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
> +                                         unsigned long *flags)
> +{
> +       const int (*parse_dt)(struct platform_device *,
> +                             struct bgpio_pdata *, unsigned long *);
> +       const struct device_node *node = pdev->dev.of_node;
> +       const struct of_device_id *of_id;
> +       struct bgpio_pdata *pdata;
> +       int err = -ENODEV;

(1)

> +
> +       of_id = of_match_node(bgpio_of_match, node);
> +       if (!of_id)
> +               return NULL;

(2)

Why so?

> +
> +       pdata = devm_kzalloc(&pdev->dev, sizeof(struct bgpio_pdata),
> +                            GFP_KERNEL);
> +       if (!pdata)
> +               return ERR_PTR(-ENOMEM);
> +
> +       parse_dt = (const void *)of_id->data;
> +       if (parse_dt)
> +               err = parse_dt(pdev, pdata, flags);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return pdata;
> +}
> +#else
> +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
> +                                         unsigned long *flags)
> +{
> +       return NULL;
> +}
> +#endif /* CONFIG_OF */



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Mark Rutland]

Hi,

On Fri, May 06, 2016 at 01:10:20PM +0200, Christian Lamparter wrote:
> +static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
> +				     struct bgpio_pdata *pdata,
> +				     unsigned long *flags)
> +{
> +	struct device *dev = &pdev->dev;
> +	int err;
> +
> +	pdata->base = -1;
> +	/* If ngpio property is not specified, of_property_read_u32
> +	 * will return -EINVAL. In this case the number of GPIOs is
> +	 * automatically determined by the register width. Any
> +	 * other error of of_property_read_u32 is due bad data and
> +	 * needs to be dealt with.
> +	 */
> +	err = of_property_read_u32(dev->of_node, "ngpio", &pdata->ngpio);
> +	if (err && err != -EINVAL)
> +		return err;
> +
> +	if (of_device_is_big_endian(dev->of_node))
> +		*flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
> +
> +	if (of_property_read_bool(dev->of_node, "unreadable-reg-set"))
> +		*flags |= BGPIOF_UNREADABLE_REG_SET;
> +
> +	if (of_property_read_bool(dev->of_node, "unreadable-reg-dir"))
> +		*flags |= BGPIOF_UNREADABLE_REG_DIR;
> +
> +	if (of_property_read_bool(dev->of_node, "big-endian-byte-order"))
> +		*flags |= BGPIOF_BIG_ENDIAN;
> +
> +	if (of_property_read_bool(dev->of_node, "read-output-reg-set"))
> +		*flags |= BGPIOF_READ_OUTPUT_REG_SET;
> +
> +	if (of_property_read_bool(dev->of_node, "no-output"))
> +		*flags |= BGPIOF_NO_OUTPUT;
> +	return 0;
> +}

Other than no-output, none of these are in the wd,mbl-gpio binding.

Please remove them for now. We can add them as/when other users of this
binding appear and begin to need them.

Thanks,
Mark.

Re: [PATCH v7 3/3] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-mmio

[Andy Shevchenko]

On Fri, May 6, 2016 at 2:10 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> This patch integrates these GPIO drivers into gpio-mmio.

Would be nice to repeat a list here.

>  config GPIO_GENERIC_PLATFORM
>         tristate "Generic memory-mapped GPIO controller support (MMIO platform device)"
>         select GPIO_GENERIC
>         help
> +         Select this to support many generic memory-mapped GPIO controllers.
> +
> +         This driver also includes support for the following GPIOs:
> +           CLPS711X SoCs
> +           MOXA ART SoC
> +           TS-4800 FPGA DIO blocks and compatibles.
> +           GPIOs found on some GE Single Board Computers.
> +
>           Say yes here to support basic platform_device memory-mapped GPIO controllers.
>
>  config GPIO_GRGPIO
> @@ -285,14 +274,6 @@ config GPIO_MM_LANTIQ
>           (EBU) found on Lantiq SoCs. The gpios are output only as they are
>           created by attaching a 16bit latch to the bus.
>
> -config GPIO_MOXART
> -       bool "MOXART GPIO support"
> -       depends on ARCH_MOXART || COMPILE_TEST
> -       select GPIO_GENERIC
> -       help
> -         Select this option to enable GPIO driver for
> -         MOXA ART SoC devices.
> -

Doesn't it change behaviour? So, as a user of old .config I expect to
have driver enabled. How is it achieved now?

> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -610,10 +610,200 @@ static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
>         return 0;
>  }
>
> +static int clps711x_parse_dt(struct platform_device *pdev,
> +                            struct bgpio_pdata *pdata,
> +                            unsigned long *flags)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct resource *res;
> +       const char *dir_reg_name;
> +       int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
> +
> +       if ((id < 0) || (id > 4))
> +               return -ENODEV;
> +

> +       /* PORTE is 3 lines only */
> +       pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0;
> +
> +       /* PORTD is inverted logic for direction register */
> +       dir_reg_name = (id == 3) ? "dirin" : "dirout",

Just a nit: possible to use switch case?

> +
> +       pdata->base = id * 8;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -EINVAL;
> +       if (!res->name || strcmp("dat", res->name))
> +               res->name = devm_kstrdup(&pdev->dev, "dat", GFP_KERNEL);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +       if (!res)
> +               return -EINVAL;
> +       if (!res->name || strcmp(dir_reg_name, res->name))
> +               res->name = devm_kstrdup(&pdev->dev, dir_reg_name, GFP_KERNEL);
> +
> +       return 0;
> +}
> +
> +static int ge_dt_cb(struct platform_device *pdev,
> +                   struct bgpio_pdata *pdata,
> +                   unsigned long *flags)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +
> +       pdata->label = devm_kstrdup(&pdev->dev, np->full_name, GFP_KERNEL);
> +       if (!pdata->label)
> +               return -ENOMEM;
> +
> +       return 0;
> +}
> +
> +static int moxart_dt_cb(struct platform_device *pdev,
> +                       struct bgpio_pdata *pdata,
> +                       unsigned long *flags)
> +{
> +       pdata->base = 0;
> +       pdata->label = "moxart-gpio";
> +       return 0;
> +}
> +
> +
> +static int ts4800_dt_cb(struct platform_device *pdev,
> +                       struct bgpio_pdata *pdata,
> +                       unsigned long *flags)
> +{
> +       int err;
> +
> +       err = of_property_read_u32(pdev->dev.of_node, "ngpios", &pdata->ngpio);
> +       if (err == -EINVAL) {
> +               pdata->ngpio = 16;
> +               err = 0;

return 0; ? (Up to you)

> +       }
> +       return err;
> +}
> +
> +struct compat_gpio_device_data {
> +       unsigned int expected_resource_size;
> +       unsigned int ngpio;
> +       resource_size_t register_width;
> +       unsigned long flags;
> +       int (*call_back)(struct platform_device *pdev,
> +                        struct bgpio_pdata *pdata,
> +                        unsigned long *flags);
> +       struct resource_replacement {
> +               resource_size_t start_offset;
> +               const char *name;
> +       } resources[5];

I would define magic number with a description what are those 5.

> +};

[...]

> +static int compat_parse_dt(struct platform_device *pdev,
> +                          struct bgpio_pdata *pdata,
> +                          unsigned long *flags)
> +{
> +       const struct device_node *node = pdev->dev.of_node;
> +       const struct compat_gpio_device_data *entry;
> +       const struct of_device_id *of_id;
> +       struct resource *res;
> +       int err;
> +
> +       of_id = of_match_node(compat_gpio_devices, node);
> +       if (!of_id)
> +               return -ENODEV;
> +
> +       entry = of_id->data;
> +       if (!entry || !entry->resources[0].name)
> +               return -EINVAL;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -EINVAL;
> +
> +       if (!res->name || strcmp(entry->resources[0].name, res->name)) {
> +               struct resource nres[ARRAY_SIZE(entry->resources)];
> +               int i;

unsigned int i; ?

> +
> +               if (resource_size(res) != entry->expected_resource_size)
> +                       return -EINVAL;
> +
> +               for (i = 0; i < ARRAY_SIZE(entry->resources); i++) {
> +                       if (!entry->resources[i].name)
> +                               continue;
> +
> +                       nres[i].name = devm_kstrdup(&pdev->dev,
> +                               entry->resources[i].name, GFP_KERNEL);
> +                       nres[i].start = res->start +
> +                               entry->resources[i].start_offset;
> +                       nres[i].end = nres[i].start +
> +                               entry->register_width - 1;
> +                       nres[i].flags = IORESOURCE_MEM;
> +               }
> +
> +               err = platform_device_add_resources(pdev, nres, i);
> +               if (err)
> +                       return err;
> +       }
> +
> +       pdata->base = -1;
> +       pdata->ngpio = entry->ngpio;
> +       *flags = entry->flags;
> +
> +       if (entry->call_back)
> +               err = entry->call_back(pdev, pdata, flags);
> +
> +       return err;
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Christian Lamparter]

On Friday, May 06, 2016 02:44:14 PM Andy Shevchenko wrote:
> > +       /* If ngpio property is not specified, of_property_read_u32
> > +        * will return -EINVAL. In this case the number of GPIOs is
> > +        * automatically determined by the register width. Any
> > +        * other error of of_property_read_u32 is due bad data and
> > +        * needs to be dealt with.
> 
> Couple style issues:
> /*
>  * First sentence starts here. func() are going with parens.
>  */
Ack, I'm used to drivers/net. I have to remove the ngpio in a future
version, so the comment is removed as well.

> > +
> > +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
> > +                                         unsigned long *flags)
> > +{
> > +       const int (*parse_dt)(struct platform_device *,
> > +                             struct bgpio_pdata *, unsigned long *);
> > +       const struct device_node *node = pdev->dev.of_node;
> > +       const struct of_device_id *of_id;
> > +       struct bgpio_pdata *pdata;
> > +       int err = -ENODEV;
> 
> (1)
> 
> > +
> > +       of_id = of_match_node(bgpio_of_match, node);
> > +       if (!of_id)
> > +               return NULL;
> 
> (2)
> 
> Why so?

This is because of existing arch code in /arch/arm/mach-clps711x/board-p720t.c.
You remember there's a driver with a device-tree binding "cirrus,clps711x-gpio"
for it. But the current kernel arch code still registers a platform_device and
it doesn't look like it has any device tree support. So this is necessary for
those partially converted archs to co-exist with the driver. I'll update the
-ENODEV to -EINVAL.

> > +
> > +       pdata = devm_kzalloc(&pdev->dev, sizeof(struct bgpio_pdata),
> > +                            GFP_KERNEL);
> > +       if (!pdata)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       parse_dt = (const void *)of_id->data;
> > +       if (parse_dt)
> > +               err = parse_dt(pdev, pdata, flags);
> > +       if (err)
> > +               return ERR_PTR(err);
> > +
> > +       return pdata;
> > +}
> > +#else
> > +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
> > +                                         unsigned long *flags)
> > +{
> > +       return NULL;
> > +}
> > +#endif /* CONFIG_OF */
--
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

Re: [PATCH v7 3/3] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-mmio

[Christian Lamparter]

On Friday, May 06, 2016 02:53:24 PM Andy Shevchenko wrote:
> On Fri, May 6, 2016 at 2:10 PM, Christian Lamparter
> <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> > This patch integrates these GPIO drivers into gpio-mmio.
> 
> Would be nice to repeat a list here.
Ok.
 
> > @@ -285,14 +274,6 @@ config GPIO_MM_LANTIQ
> >           (EBU) found on Lantiq SoCs. The gpios are output only as they are
> >           created by attaching a 16bit latch to the bus.
> >
> > -config GPIO_MOXART
> > -       bool "MOXART GPIO support"
> > -       depends on ARCH_MOXART || COMPILE_TEST
> > -       select GPIO_GENERIC
> > -       help
> > -         Select this option to enable GPIO driver for
> > -         MOXA ART SoC devices.
> > -
> 
> Doesn't it change behaviour? So, as a user of old .config I expect to
> have driver enabled. How is it achieved now?
Ok, I'll restore the configs and let them select GPIO_GENERIC_PLATFORM.

> > --- a/drivers/gpio/gpio-mmio.c
> > +++ b/drivers/gpio/gpio-mmio.c
> > @@ -610,10 +610,200 @@ static int bgpio_basic_mmio_parse_dt(struct platform_device *pdev,
> >         return 0;
> >  }
> >
> > +static int clps711x_parse_dt(struct platform_device *pdev,
> > +                            struct bgpio_pdata *pdata,
> > +                            unsigned long *flags)
> > +{
> > +       struct device_node *np = pdev->dev.of_node;
> > +       struct resource *res;
> > +       const char *dir_reg_name;
> > +       int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
> > +
> > +       if ((id < 0) || (id > 4))
> > +               return -ENODEV;
> > +
> 
> > +       /* PORTE is 3 lines only */
> > +       pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0;
> > +
> > +       /* PORTD is inverted logic for direction register */
> > +       dir_reg_name = (id == 3) ? "dirin" : "dirout",
Actually, the , should be a ; it compiled because , is an operator.
But yes, fixed.

> Just a nit: possible to use switch case?
I think so. I've integrated the id < 0 || id > 4 check as well.
I don't know, I liked the old version more, it was shorter

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index f116786..f71021c 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -619,14 +619,25 @@ static int clps711x_parse_dt(struct platform_device *pdev,
 	const char *dir_reg_name;
 	int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
 
-	if ((id < 0) || (id > 4))
+	switch (id) {
+	case 0:
+	case 1:
+	case 2:
+		pdata->ngpio = 0; /* determined by register width */
+		dir_reg_name = "dirout";
+		break;
+	case 3:
+		pdata->ngpio = 0; /* determined by register width */
+		/* PORTD is inverted logic for direction register */
+		dir_reg_name = "dirin";
+		break;
+	case 4:
+		pdata->ngpio = 3; /* PORTE is 3 lines only */
+		dir_reg_name = "dirout";
+		break;
+	default:
 		return -ENODEV;
-
-	/* PORTE is 3 lines only */
-	pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0;
-
-	/* PORTD is inverted logic for direction register */
-	dir_reg_name = (id == 3) ? "dirin" : "dirout",
+	}
 
 	pdata->base = id * 8;
---
  
> > +
> > +       pdata->base = id * 8;
> > +
> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       if (!res)
> > +               return -EINVAL;
> > +       if (!res->name || strcmp("dat", res->name))
> > +               res->name = devm_kstrdup(&pdev->dev, "dat", GFP_KERNEL);
> > +
> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > +       if (!res)
> > +               return -EINVAL;
> > +       if (!res->name || strcmp(dir_reg_name, res->name))
> > +               res->name = devm_kstrdup(&pdev->dev, dir_reg_name, GFP_KERNEL);
> > +
> > +       return 0;
> > +}

[...]

> > +struct compat_gpio_device_data {
> > +       unsigned int expected_resource_size;
> > +       unsigned int ngpio;
> > +       resource_size_t register_width;
> > +       unsigned long flags;
> > +       int (*call_back)(struct platform_device *pdev,
> > +                        struct bgpio_pdata *pdata,
> > +                        unsigned long *flags);
> > +       struct resource_replacement {
> > +               resource_size_t start_offset;
> > +               const char *name;
> > +       } resources[5];
> 
> I would define magic number with a description what are those 5.

Difficult, these are 5 placeholders for the named resources.
There's particular order so a enum like
{
	DAT,
	SET,
	CLR,
	DIRIN,
	DIROUT,
	__NUM_RES 
}

might look nice but it has no purpose other than maybe confusing people
why the entries aren't used.

> > +};
> 
> [...]
> 
> > +static int compat_parse_dt(struct platform_device *pdev,
> > +                          struct bgpio_pdata *pdata,
> > +                          unsigned long *flags)
> > +{
> > +       const struct device_node *node = pdev->dev.of_node;
> > +       const struct compat_gpio_device_data *entry;
> > +       const struct of_device_id *of_id;
> > +       struct resource *res;
> > +       int err;
> > +
> > +       of_id = of_match_node(compat_gpio_devices, node);
> > +       if (!of_id)
> > +               return -ENODEV;
> > +
> > +       entry = of_id->data;
> > +       if (!entry || !entry->resources[0].name)
> > +               return -EINVAL;
> > +
> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       if (!res)
> > +               return -EINVAL;
> > +
> > +       if (!res->name || strcmp(entry->resources[0].name, res->name)) {
> > +               struct resource nres[ARRAY_SIZE(entry->resources)];
> > +               int i;
> 
> unsigned int i; ?
yes. I think I change it to size_t. ARRAY_SIZE uses sizeof internally.
 
Regards,
Christian
--
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

Re: [PATCH v7 3/3] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-mmio

[Andy Shevchenko]

On Fri, May 6, 2016 at 4:06 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Friday, May 06, 2016 02:53:24 PM Andy Shevchenko wrote:
>> On Fri, May 6, 2016 at 2:10 PM, Christian Lamparter
>> <chunkeey@googlemail.com> wrote:

>> Just a nit: possible to use switch case?
> I think so. I've integrated the id < 0 || id > 4 check as well.
> I don't know, I liked the old version more, it was shorter
>
> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
> index f116786..f71021c 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -619,14 +619,25 @@ static int clps711x_parse_dt(struct platform_device *pdev,
>         const char *dir_reg_name;
>         int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
>
> -       if ((id < 0) || (id > 4))
> +       switch (id) {
> +       case 0:
> +       case 1:
> +       case 2:
> +               pdata->ngpio = 0; /* determined by register width */
> +               dir_reg_name = "dirout";
> +               break;
> +       case 3:
> +               pdata->ngpio = 0; /* determined by register width */
> +               /* PORTD is inverted logic for direction register */
> +               dir_reg_name = "dirin";
> +               break;
> +       case 4:
> +               pdata->ngpio = 3; /* PORTE is 3 lines only */
> +               dir_reg_name = "dirout";
> +               break;
> +       default:
>                 return -ENODEV;
> -
> -       /* PORTE is 3 lines only */
> -       pdata->ngpio = (id == 4) ? 3 : /* determined by register width */ 0;
> -
> -       /* PORTD is inverted logic for direction register */
> -       dir_reg_name = (id == 3) ? "dirin" : "dirout",
> +       }

For my personal taste the switch case more understandable / readable.
But i'm not totally objecting the old model, just a subject to discuss.

>> > +struct compat_gpio_device_data {
>> > +       unsigned int expected_resource_size;
>> > +       unsigned int ngpio;
>> > +       resource_size_t register_width;
>> > +       unsigned long flags;
>> > +       int (*call_back)(struct platform_device *pdev,
>> > +                        struct bgpio_pdata *pdata,
>> > +                        unsigned long *flags);
>> > +       struct resource_replacement {
>> > +               resource_size_t start_offset;
>> > +               const char *name;
>> > +       } resources[5];
>>
>> I would define magic number with a description what are those 5.
>
> Difficult, these are 5 placeholders for the named resources.
> There's particular order so a enum like
> {
>         DAT,
>         SET,
>         CLR,
>         DIRIN,
>         DIROUT,
>         __NUM_RES
> }
>
> might look nice but it has no purpose other than maybe confusing people
> why the entries aren't used.

OK, might be just

#define …_SUPPORTED_RES_MAX 5
…
} resources[_SUPPORTED_RES_MAX];

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html