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

[PATCH v8 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 third patch (which can be applied at a later time) moves gpio-clps711x,
gpio-ge, gpio-moxart and gpio-ts4800 into gpio-mmio.c. The old driver files
and makefile entries have been removed. The Kconfig entries for these
drivers are kept for compatibility reasons.

And finally, the most important stat about the series:
	>>> 336 insertions(+), 380 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:

v7 -> v8:
	- removed ngpio property parser and most flags parsers
	- converted clps711x to switch case for GPIO
	- retained original Kconfig syms which now select GPIO_GENERIC_PLATFORM

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                               |  16 +-
 drivers/gpio/Makefile                              |   4 -
 drivers/gpio/gpio-clps711x.c                       |  91 -------
 drivers/gpio/gpio-ge.c                             | 114 --------
 drivers/gpio/gpio-mmio.c                           | 288 ++++++++++++++++++++-
 drivers/gpio/gpio-moxart.c                         |  84 ------
 drivers/gpio/gpio-ts4800.c                         |  81 ------
 8 files changed, 336 insertions(+), 380 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

--
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

[PATCH v8 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 v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Christian Lamparter]

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".

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/gpio-mmio.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 6c1cb3b..4b7b74c 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,66 @@ 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 (of_property_read_bool(dev->of_node, "no-output"))
+		*flags |= BGPIOF_NO_OUTPUT;
+
+	return 0;
+}
+
+#define ADD(_name, _func) { .compatible = _name, .data = _func }
+
+static const struct of_device_id bgpio_of_match[] = {
+	ADD("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
+	{ }
+};
+#undef ADD
+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 +641,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 +717,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 linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Christian Lamparter]

This patch integrates the GPIO drivers for the following
boards, SoCs, etc. into gpio-mmio:
 - CLPS711X SoCs
 - MOXA ART SoC
 - TS-4800 FPGA DIO blocks and compatibles
 - GPIO controllers found on some GE Single Board Computers

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         |  16 +++-
 drivers/gpio/Makefile        |   4 -
 drivers/gpio/gpio-clps711x.c |  91 -------------------
 drivers/gpio/gpio-ge.c       | 114 -----------------------
 drivers/gpio/gpio-mmio.c     | 212 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpio-moxart.c   |  84 -----------------
 drivers/gpio/gpio-ts4800.c   |  81 -----------------
 7 files changed, 224 insertions(+), 378 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..e4d1065 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -154,7 +154,7 @@ config GPIO_BRCMSTB
 config GPIO_CLPS711X
 	tristate "CLPS711X GPIO support"
 	depends on ARCH_CLPS711X || COMPILE_TEST
-	select GPIO_GENERIC
+	select GPIO_GENERIC_PLATFORM
 	help
 	  Say yes here to support GPIO on CLPS711X SoCs.
 
@@ -196,7 +196,7 @@ config GPIO_ETRAXFS
 config GPIO_GE_FPGA
 	bool "GE FPGA based GPIO"
 	depends on GE_FPGA
-	select GPIO_GENERIC
+	select GPIO_GENERIC_PLATFORM
 	help
 	  Support for common GPIO functionality provided on some GE Single Board
 	  Computers.
@@ -209,6 +209,14 @@ 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
@@ -288,7 +296,7 @@ config GPIO_MM_LANTIQ
 config GPIO_MOXART
 	bool "MOXART GPIO support"
 	depends on ARCH_MOXART || COMPILE_TEST
-	select GPIO_GENERIC
+	select GPIO_GENERIC_PLATFORM
 	help
 	  Select this option to enable GPIO driver for
 	  MOXA ART SoC devices.
@@ -408,7 +416,7 @@ config GPIO_TS4800
 	tristate "TS-4800 DIO blocks and compatibles"
 	depends on OF_GPIO
 	depends on SOC_IMX51 || COMPILE_TEST
-	select GPIO_GENERIC
+	select GPIO_GENERIC_PLATFORM
 	help
 	  This driver support TS-4800 FPGA GPIO controllers.
 
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 4b7b74c..d1a7666 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -587,10 +587,222 @@ 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;
+
+	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;
+	}
+
+	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;
+		return 0;
+	}
+
+	return err;
+}
+
+/*
+ * bgpio_init() needs up to five named mmio register resources.
+ * These currently are dat, set, clr, [dirout | dirin]. There
+ * is no particular order or predefined place for the entries.
+ * However, dirout and dirin are mutually exclusive.
+ */
+#define __NUM_COMPAT_OVERRIDE_MMIO_RESOURCES 4
+
+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[__NUM_COMPAT_OVERRIDE_MMIO_RESOURCES];
+};
+#undef __NUM_COMPAT_OVERRIDE_MMIO_RESOURCES
+
+#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)];
+		size_t 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(_name, _func) { .compatible = _name, .data = _func }
 
 static const struct of_device_id bgpio_of_match[] = {
 	ADD("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
+	ADD("cirrus,clps711x-gpio", clps711x_parse_dt),
+	ADD("ge,imp3a-gpio", compat_parse_dt),
+	ADD("gef,sbc310-gpio", compat_parse_dt),
+	ADD("gef,sbc610-gpio", compat_parse_dt),
+	ADD("moxa,moxart-gpio", compat_parse_dt),
+	ADD("technologic,ts4800-gpio", compat_parse_dt),
 	{ }
 };
 #undef ADD
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 v8 0/3] gpio: add DT support for memory-mapped GPIOs

[Andy Shevchenko]

On Sun, May 8, 2016 at 4:08 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> 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 third patch (which can be applied at a later time) moves gpio-clps711x,
> gpio-ge, gpio-moxart and gpio-ts4800 into gpio-mmio.c. The old driver files
> and makefile entries have been removed. The Kconfig entries for these
> drivers are kept for compatibility reasons.
>
> And finally, the most important stat about the series:
>         >>> 336 insertions(+), 380 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>
>

FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> changelog:
>
> v7 -> v8:
>         - removed ngpio property parser and most flags parsers
>         - converted clps711x to switch case for GPIO
>         - retained original Kconfig syms which now select GPIO_GENERIC_PLATFORM
>
> 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                               |  16 +-
>  drivers/gpio/Makefile                              |   4 -
>  drivers/gpio/gpio-clps711x.c                       |  91 -------
>  drivers/gpio/gpio-ge.c                             | 114 --------
>  drivers/gpio/gpio-mmio.c                           | 288 ++++++++++++++++++++-
>  drivers/gpio/gpio-moxart.c                         |  84 ------
>  drivers/gpio/gpio-ts4800.c                         |  81 ------
>  8 files changed, 336 insertions(+), 380 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
>



-- 
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 v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Joachim Eastwood]

Hi Christian,

On 8 May 2016 at 15:08, 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".
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---

> +#define ADD(_name, _func) { .compatible = _name, .data = _func }

I don't see the point in having a macro for such a simple data
structure, but since this v8 and Linus hasn't complained I guess it's
fine.

Using a macro here makes it impossible to grep for 'compatible'. Doing
'git grep compatible drivers/gpio/' is sometimes very useful to see
which hardware the driver actually supports.

> +static const struct of_device_id bgpio_of_match[] = {
> +       ADD("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
> +       { }
> +};
> +#undef ADD
> +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;

You can retrieve OF match data using of_device_get_match_data(). Saves
you a couple of lines and better explains what your doing.


> +       if (parse_dt)
> +               err = parse_dt(pdev, pdata, flags);
> +       if (err)
> +               return ERR_PTR(err);
> +
> +       return pdata;
> +}


regards,
Joachim Eastwood
--
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 v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Christian Lamparter]

On Sunday, May 08, 2016 07:17:13 PM Joachim Eastwood wrote:
> > +#define ADD(_name, _func) { .compatible = _name, .data = _func }
> 
> I don't see the point in having a macro for such a simple data
> structure, but since this v8 and Linus hasn't complained I guess it's
> fine.
>
> Using a macro here makes it impossible to grep for 'compatible'. Doing
> 'git grep compatible drivers/gpio/' is sometimes very useful to see
> which hardware the driver actually supports.

Ok. I'll definitely picking it up. I'll wait until Tuesday/Wednesday
for more comments and then release a new series.
 
> > +static const struct of_device_id bgpio_of_match[] = {
> > +       ADD("wd,mbl-gpio", bgpio_basic_mmio_parse_dt),
> > +       { }
> > +};
> > +#undef ADD
> > +MODULE_DEVICE_TABLE(of, bgpio_of_match);
> > +
> > +static struct bgpio_pdata *bgpio_parse_dt(struct platform_device *pdev,
> > +                                         unsigned long *flags)
> > +{
[...]
> > +       of_id = of_match_node(bgpio_of_match, node);
> > +       if (!of_id)
> > +               return NULL;
[...]
> You can retrieve OF match data using of_device_get_match_data().
> Saves you a couple of lines and better explains what your doing.
Yes thanks that's useful too since I don't need the of_id variable
anymore.

Both improvements save a total of 8 lines. so it's 328 insertions(+)
vs 380 deletions(-).

Regards,
Christian

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

[Linus Walleij]

On Sun, May 8, 2016 at 3:08 PM, Christian Lamparter
<chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:

> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

Looks OK to me and Rob has ACKed so patch applied.

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

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

[Linus Walleij]

On Sun, May 8, 2016 at 3:08 PM, Christian Lamparter
<chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:

> 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.

Overall very nice, just waiting for the next version.

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

And that binding defines that we have a register named "dat".

> +       if (of_property_read_bool(dev->of_node, "no-output"))

And then this too.

Do we want these generic MMIO bindings (dat, no-output)
in a special document like
Documentation/devicetree/bindings/gpio/gpio-mmio.txt?

Going forward?

This patch set mainly deals with refactorings, but in the
long run we want to slim things down a bit and use standard
bindings I 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

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

[Christian Lamparter]

On Tuesday, May 10, 2016 02:08:45 PM Linus Walleij wrote:
> On Sun, May 8, 2016 at 3:08 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.
> 
> Overall very nice, just waiting for the next version.

K, will deliver. I noticed that you sent a mail in which you stated
that you applied the dt binding already. Can you update your devel
branch on git.kernel.org's linux-gpio? Then, I'll simply rebase my
series and sent the remaining two patches.
(unless you tell me otherwise).
 
> > The first user for this binding is "wd,mbl-gpio".
> 
> And that binding defines that we have a register named "dat".

Yeah, I had to remove all non wd related bits. But since this 
series was posted (over and over :D) on a public mailing-list
the original  "generic" linux-mmio binding is available for
everybody to perusal[0] and study. I think what we can make
would be something like a devicetree template out of it. This
way people can remove unused flags and regnames for their
compatible device tree binding. (But first: need to finish
that ppc-gpio.txt).

> > +       if (of_property_read_bool(dev->of_node, "no-output"))
> 
> And then this too.
> 
> Do we want these generic MMIO bindings (dat, no-output)
> in a special document like
> Documentation/devicetree/bindings/gpio/gpio-mmio.txt?
> 
> Going forward?
Ah, I was thinking about Documentation/gpio... Since there's no
way it would go in the devicetree/bindings without having a compatible?
(And there's technically none). As far as I know the problem here is not
that it would be impossible to do that (updating a .dts file is "easy"...),
but updating .dtb to a tiny flash-rom on the device might not be. So we
have to make every effort to preserve compatibility for those devices
(and old/incomplete/broken dtbs) as long as the device is supported.

About adding new device:
This will work in the following way: 
 1. new drives will need to supply their hardware-specific devicetree
    binding file to the dt maintainers (This "vendor,device.txt" file
    will be like the wd,mbl-gpio.txt - but modified for the hardware
    (this is where the template would be handy)

 2. Make a one-liner patch which adds a compatible string to
    gpio-mmio.c's bgpio_of_match table:

+	{ .compatible = "vendor,device", .data = bgpio_basic_mmio_parse_dt },

(Of course, not having parses for the "ngpio" property and the flags like
big-endian, reg-output-reg,set, unreadable-reg-dir, ... properties from
the get-go is sad, these can add back once a driver/binding needs it).

I think brcm63xx will be following shortly. So we can test the
procedure.
> This patch set mainly deals with refactorings, but in the
> long run we want to slim things down a bit and use standard
> bindings I think.
Well, to do that, I think we need to collect enough devices to
make it a real "class" of devices first.

Regards,
Christian

[0] <https://lkml.org/lkml/2016/4/28/921>
--
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 v8 2/3] gpio: mmio: add DT support for memory-mapped GPIOs

[Álvaro Fernández Rojas]

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

Hello everyone,

First of all thanks Christian for persisting in adding generic device tree support for memory-mapped gpios.

For the brcm63xx case we could probably do something like this:
https://gist.github.com/Noltari/fd34aedafd427d25467697ec6799d15a
Notice that I'm doing something similar to what's already done for non device tree with basic-mmio-gpio vs basic-mmio-gpio-be:
https://github.com/torvalds/linux/blob/master/drivers/gpio/gpio-generic.c#L634

P.S: excuse me for not using an interleaved reply, but I'm sending this from my phone :)

Regards,
Álvaro.


El 10 may 2016 21:55 +0200, Christian Lamparter<chunkeey@googlemail.com>, escribió:
> On Tuesday, May 10, 2016 02:08:45 PM Linus Walleij wrote:
> > On Sun, May 8, 2016 at 3:08 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.
> >  
> > Overall very nice, just waiting for the next version.
>  
> K, will deliver. I noticed that you sent a mail in which you stated
> that you applied the dt binding already. Can you update your devel
> branch on git.kernel.org's linux-gpio? Then, I'll simply rebase my
> series and sent the remaining two patches.
> (unless you tell me otherwise).
>  
> > > The first user for this binding is "wd,mbl-gpio".
> >  
> > And that binding defines that we have a register named "dat".
>  
> Yeah, I had to remove all non wd related bits. But since this
> series was posted (over and over :D) on a public mailing-list
> the original "generic" linux-mmio binding is available for
> everybody to perusal[0] and study. I think what we can make
> would be something like a devicetree template out of it. This
> way people can remove unused flags and regnames for their
> compatible device tree binding. (But first: need to finish
> that ppc-gpio.txt).
>  
> > > + if (of_property_read_bool(dev->of_node, "no-output"))
> >  
> > And then this too.
> >  
> > Do we want these generic MMIO bindings (dat, no-output)
> > in a special document like
> > Documentation/devicetree/bindings/gpio/gpio-mmio.txt?
> >  
> > Going forward?
> Ah, I was thinking about Documentation/gpio... Since there's no
> way it would go in the devicetree/bindings without having a compatible?
> (And there's technically none). As far as I know the problem here is not
> that it would be impossible to do that (updating a .dts file is "easy"...),
> but updating .dtb to a tiny flash-rom on the device might not be. So we
> have to make every effort to preserve compatibility for those devices
> (and old/incomplete/broken dtbs) as long as the device is supported.
>  
> About adding new device:
> This will work in the following way:
> 1. new drives will need to supply their hardware-specific devicetree
> binding file to the dt maintainers (This "vendor,device.txt" file
> will be like the wd,mbl-gpio.txt - but modified for the hardware
> (this is where the template would be handy)
>  
> 2. Make a one-liner patch which adds a compatible string to
> gpio-mmio.c's bgpio_of_match table:
>  
> + { .compatible = "vendor,device", .data = bgpio_basic_mmio_parse_dt },
>  
> (Of course, not having parses for the "ngpio" property and the flags like
> big-endian, reg-output-reg,set, unreadable-reg-dir, ... properties from
> the get-go is sad, these can add back once a driver/binding needs it).
>  
> I think brcm63xx will be following shortly. So we can test the
> procedure.
> > This patch set mainly deals with refactorings, but in the
> > long run we want to slim things down a bit and use standard
> > bindings I think.
> Well, to do that, I think we need to collect enough devices to
> make it a real "class" of devices first.
>  
> Regards,
> Christian
>  
> [0]<https://lkml.org/lkml/2016/4/28/921

[-- Attachment #2: Type: text/html, Size: 4868 bytes --]