[RFC v5 0/4] gpio: add DT support for generic memory-mapped GPIOs

[RFC v5 0/4] gpio: add DT support for generic 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 - ppc464). This generic approach patch
allowed me to easily get the GPIO (and the connected LEDs,
buttons, gpiohogs, etc.) up and running. Even tought, Mr. Fernandez
initially developed it for his work on the brcm63xx [2].

The device tree parses for the integrated drivers:
gpio-clps711x, gpio-ge, gpio-moxart and gpio-ts4800 are now part
of the gpio-generic.c driver file. The old driver files have been
removed and the Kconfig, Makefile entries have been updated
accordingly. If no complaints are filed (MODULE_ALIASES?), I'll
go ahead and post the PATCH series beginning of next week.

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

Thanks! (Please keep me in the CC)

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

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: generic: fix GPIO_GENERIC_PLATFORM is set to module case
  gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

Álvaro Fernández Rojas (2):
  gpio: generic: add DT support for generic memory-mapped GPIOs
  gpio: dt-bindings: add gpio-mmio bindings

 .../devicetree/bindings/gpio/gpio-mmio.txt         |  73 +++++++
 drivers/gpio/Kconfig                               |  43 +---
 drivers/gpio/Makefile                              |   4 -
 drivers/gpio/gpio-clps711x.c                       |  91 --------
 drivers/gpio/gpio-ge.c                             | 114 ----------
 drivers/gpio/gpio-generic.c                        | 234 ++++++++++++++++++++-
 drivers/gpio/gpio-moxart.c                         |  84 --------
 drivers/gpio/gpio-ts4800.c                         |  81 -------
 8 files changed, 312 insertions(+), 412 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.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

[RFC v5 0/4] gpio: add DT support for generic 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 - ppc464). This generic approach patch
allowed me to easily get the GPIO (and the connected LEDs,
buttons, gpiohogs, etc.) up and running. Even tought, Mr. Fernandez
initially developed it for his work on the brcm63xx [2].

The device tree parses for the integrated drivers:
gpio-clps711x, gpio-ge, gpio-moxart and gpio-ts4800 are now part
of the gpio-generic.c driver file. The old driver files have been
removed and the Kconfig, Makefile entries have been updated
accordingly. If no complaints are filed (MODULE_ALIASES?), I'll
go ahead and post the PATCH series beginning of next week.

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

Thanks! (Please keep me in the CC)

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

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: generic: fix GPIO_GENERIC_PLATFORM is set to module case
  gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

?lvaro Fern?ndez Rojas (2):
  gpio: generic: add DT support for generic memory-mapped GPIOs
  gpio: dt-bindings: add gpio-mmio bindings

 .../devicetree/bindings/gpio/gpio-mmio.txt         |  73 +++++++
 drivers/gpio/Kconfig                               |  43 +---
 drivers/gpio/Makefile                              |   4 -
 drivers/gpio/gpio-clps711x.c                       |  91 --------
 drivers/gpio/gpio-ge.c                             | 114 ----------
 drivers/gpio/gpio-generic.c                        | 234 ++++++++++++++++++++-
 drivers/gpio/gpio-moxart.c                         |  84 --------
 drivers/gpio/gpio-ts4800.c                         |  81 -------
 8 files changed, 312 insertions(+), 412 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.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

[RFC v5 0/4] gpio: add DT support for generic 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 - ppc464). This generic approach patch
allowed me to easily get the GPIO (and the connected LEDs,
buttons, gpiohogs, etc.) up and running. Even tought, Mr. Fernandez
initially developed it for his work on the brcm63xx [2].

The device tree parses for the integrated drivers:
gpio-clps711x, gpio-ge, gpio-moxart and gpio-ts4800 are now part
of the gpio-generic.c driver file. The old driver files have been
removed and the Kconfig, Makefile entries have been updated
accordingly. If no complaints are filed (MODULE_ALIASES?), I'll
go ahead and post the PATCH series beginning of next week.

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

Thanks! (Please keep me in the CC)

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

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: generic: fix GPIO_GENERIC_PLATFORM is set to module case
  gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

Álvaro Fernández Rojas (2):
  gpio: generic: add DT support for generic memory-mapped GPIOs
  gpio: dt-bindings: add gpio-mmio bindings

 .../devicetree/bindings/gpio/gpio-mmio.txt         |  73 +++++++
 drivers/gpio/Kconfig                               |  43 +---
 drivers/gpio/Makefile                              |   4 -
 drivers/gpio/gpio-clps711x.c                       |  91 --------
 drivers/gpio/gpio-ge.c                             | 114 ----------
 drivers/gpio/gpio-generic.c                        | 234 ++++++++++++++++++++-
 drivers/gpio/gpio-moxart.c                         |  84 --------
 drivers/gpio/gpio-ts4800.c                         |  81 -------
 8 files changed, 312 insertions(+), 412 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.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

[RFC v5 1/4] gpio: generic: fix GPIO_GENERIC_PLATFORM is set to module case

[Christian Lamparter]

GPIO_GENERIC_PLATFORM is a tristate. If the module option is
selected the resulting gpio-generic.ko will lack most of the
module initialzation and probe code.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/gpio-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 54cddfa..6c1cb3b 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -549,7 +549,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
 }
 EXPORT_SYMBOL_GPL(bgpio_init);
 
-#ifdef CONFIG_GPIO_GENERIC_PLATFORM
+#if IS_ENABLED(CONFIG_GPIO_GENERIC_PLATFORM)
 
 static void __iomem *bgpio_map(struct platform_device *pdev,
 			       const char *name,
-- 
2.8.1

[RFC v5 1/4] gpio: generic: fix GPIO_GENERIC_PLATFORM is set to module case

[Christian Lamparter]

GPIO_GENERIC_PLATFORM is a tristate. If the module option is
selected the resulting gpio-generic.ko will lack most of the
module initialzation and probe code.

Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/gpio-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 54cddfa..6c1cb3b 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -549,7 +549,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
 }
 EXPORT_SYMBOL_GPL(bgpio_init);
 
-#ifdef CONFIG_GPIO_GENERIC_PLATFORM
+#if IS_ENABLED(CONFIG_GPIO_GENERIC_PLATFORM)
 
 static void __iomem *bgpio_map(struct platform_device *pdev,
 			       const char *name,
-- 
2.8.1

Re: [RFC v5 1/4] gpio: generic: fix GPIO_GENERIC_PLATFORM is set to module case

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> GPIO_GENERIC_PLATFORM is a tristate. If the module option is
> selected the resulting gpio-generic.ko will lack most of the
> module initialzation and probe code.
>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

Obviously correct, patch applied.

Yours,
Linus Walleij

[RFC v5 1/4] gpio: generic: fix GPIO_GENERIC_PLATFORM is set to module case

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> GPIO_GENERIC_PLATFORM is a tristate. If the module option is
> selected the resulting gpio-generic.ko will lack most of the
> module initialzation and probe code.
>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

Obviously correct, patch applied.

Yours,
Linus Walleij

[RFC v5 2/4] gpio: generic: add DT support for generic memory-mapped GPIOs

[Christian Lamparter]

From: Álvaro Fernández Rojas <noltari@gmail.com>

This patch adds support for defining memory-mapped GPIOs
which provide a compatible interface for the existing
generic-gpio driver.

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

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 6c1cb3b..129c3ba 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.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,88 @@ 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;
+
+	/* 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(_name, _func) { .compatible = _name, .data = _func }
+
+static const struct of_device_id bgpio_of_match[] = {
+	ADD("linux,gpio-mmio", 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)
+{
+	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 = 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 +663,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 +739,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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[RFC v5 2/4] gpio: generic: add DT support for generic memory-mapped GPIOs

[Christian Lamparter]

From: ?lvaro Fern?ndez Rojas <noltari@gmail.com>

This patch adds support for defining memory-mapped GPIOs
which provide a compatible interface for the existing
generic-gpio driver.

Signed-off-by: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 drivers/gpio/gpio-generic.c | 98 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 6c1cb3b..129c3ba 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.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,88 @@ 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;
+
+	/* 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(_name, _func) { .compatible = _name, .data = _func }
+
+static const struct of_device_id bgpio_of_match[] = {
+	ADD("linux,gpio-mmio", 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)
+{
+	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 = 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 +663,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 +739,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

[RFC v5 2/4] gpio: generic: add DT support for generic memory-mapped GPIOs

[Christian Lamparter]

From: Álvaro Fernández Rojas <noltari@gmail.com>

This patch adds support for defining memory-mapped GPIOs
which provide a compatible interface for the existing
generic-gpio driver.

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

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 6c1cb3b..129c3ba 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.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,88 @@ 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;
+
+	/* 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(_name, _func) { .compatible = _name, .data = _func }
+
+static const struct of_device_id bgpio_of_match[] = {
+	ADD("linux,gpio-mmio", 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)
+{
+	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 = 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 +663,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 +739,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

[RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Christian Lamparter]

This patch integrates these GPIO drivers into gpio-generic.

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-generic.c  | 134 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpio-moxart.c   |  84 ---------------------------
 drivers/gpio/gpio-ts4800.c   |  81 --------------------------
 7 files changed, 142 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 f73f26b..df6ca48 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -152,13 +152,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
@@ -194,22 +187,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
@@ -286,14 +275,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
@@ -405,14 +386,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 74eb1a7..f3856df 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -28,7 +28,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
@@ -40,7 +39,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
@@ -65,7 +63,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
@@ -104,7 +101,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-generic.c b/drivers/gpio/gpio-generic.c
index 129c3ba..54afd44 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -609,10 +609,144 @@ 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;
+	int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
+	struct resource *res;
+
+	if ((id < 0) || (id > 4))
+		return -ENODEV;
+
+	/* PORTE is 3 lines only */
+	if (id == 4)
+		pdata->ngpio = 3;
+
+	pdata->base = id * 8;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	res->name = "dat";
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -EINVAL;
+
+	/* PORTD is inverted logic for direction register */
+	res->name = (id == 3) ? "dirin" : "dirout";
+	return 0;
+}
+
+static int ge_parse_dt(struct platform_device *pdev,
+		       struct bgpio_pdata *pdata,
+		       unsigned long *flags)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	struct resource *res;
+	struct resource nres[] = {
+		DEFINE_RES_MEM_NAMED(0, 1, "dat"),
+		DEFINE_RES_MEM_NAMED(0, 1, "set"),
+		DEFINE_RES_MEM_NAMED(0, 1, "dirin"),
+	};
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res || resource_size(res) != 0x24)
+		return -EINVAL;
+
+	nres[0].start = res->start + 0x04;
+	nres[0].end = nres[0].start + 3;
+	nres[1].start = res->start + 0x08;
+	nres[1].end = nres[1].start + 3;
+	nres[2].start = res->start + 0x00;
+	nres[2].end = nres[2].start + 3;
+	*flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+	pdata->label = devm_kstrdup(&pdev->dev, np->full_name, GFP_KERNEL);
+	if (!pdata->label)
+		return -ENOMEM;
+
+	if (of_device_is_compatible(np, "ge,imp3a-gpio"))
+		pdata->ngpio = 16;
+	else if (of_device_is_compatible(np, "gef,sbc310-gpio"))
+		pdata->ngpio = 6;
+	else if (of_device_is_compatible(np, "gef,sbc610-gpio"))
+		pdata->ngpio = 19;
+
+	return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
+}
+
+static int moxart_parse_dt(struct platform_device *pdev,
+			   struct bgpio_pdata *pdata,
+			   unsigned long *flags)
+{
+	struct resource *res;
+	struct resource nres[] = {
+		DEFINE_RES_MEM_NAMED(0, 1, "dat"),
+		DEFINE_RES_MEM_NAMED(0, 1, "set"),
+		DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
+	};
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res || resource_size(res) != 0xc)
+		return -EINVAL;
+
+	pdata->label = "moxart-gpio";
+	*flags |= BGPIOF_READ_OUTPUT_REG_SET;
+	nres[0].start = res->start + 0x04;
+	nres[0].end = nres[0].start + 3;
+	nres[1].start = res->start + 0x00;
+	nres[1].end = nres[1].start + 3;
+	nres[2].start = res->start + 0x08;
+	nres[2].end = nres[2].start + 3;
+	return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
+}
+
+static int ts4800_parse_dt(struct platform_device *pdev,
+			   struct bgpio_pdata *pdata,
+			   unsigned long *flags)
+{
+	int err;
+	struct resource *res;
+	struct resource nres[] = {
+		DEFINE_RES_MEM_NAMED(0, 1, "dat"),
+		DEFINE_RES_MEM_NAMED(0, 1, "set"),
+		DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
+	};
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res || resource_size(res) != 0x6)
+		return -EINVAL;
+
+	nres[0].start = res->start;
+	nres[0].end = nres[0].start + 1;
+	nres[1].start = res->start + 0x02;
+	nres[1].end = nres[1].start + 1;
+	nres[2].start = res->start + 0x04;
+	nres[2].end = nres[2].start + 1;
+
+	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &pdata->ngpio);
+	if (err == -EINVAL)
+		pdata->ngpio = 16;
+	else if (err)
+		return err;
+
+	return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
+}
+
 #define ADD(_name, _func) { .compatible = _name, .data = _func }
 
 static const struct of_device_id bgpio_of_match[] = {
 	ADD("linux,gpio-mmio", bgpio_basic_mmio_parse_dt),
+	ADD("cirrus,clps711x-gpio", clps711x_parse_dt),
+	ADD("ge,imp3a-gpio", ge_parse_dt),
+	ADD("gef,sbc310-gpio", ge_parse_dt),
+	ADD("gef,sbc610-gpio", ge_parse_dt),
+	ADD("moxa,moxart-gpio", moxart_parse_dt),
+	ADD("technologic,ts4800-gpio", ts4800_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

[RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Christian Lamparter]

This patch integrates these GPIO drivers into gpio-generic.

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-generic.c  | 134 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpio-moxart.c   |  84 ---------------------------
 drivers/gpio/gpio-ts4800.c   |  81 --------------------------
 7 files changed, 142 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 f73f26b..df6ca48 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -152,13 +152,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
@@ -194,22 +187,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
@@ -286,14 +275,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
@@ -405,14 +386,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 74eb1a7..f3856df 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -28,7 +28,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
@@ -40,7 +39,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
@@ -65,7 +63,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
@@ -104,7 +101,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-generic.c b/drivers/gpio/gpio-generic.c
index 129c3ba..54afd44 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -609,10 +609,144 @@ 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;
+	int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
+	struct resource *res;
+
+	if ((id < 0) || (id > 4))
+		return -ENODEV;
+
+	/* PORTE is 3 lines only */
+	if (id == 4)
+		pdata->ngpio = 3;
+
+	pdata->base = id * 8;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -EINVAL;
+
+	res->name = "dat";
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res)
+		return -EINVAL;
+
+	/* PORTD is inverted logic for direction register */
+	res->name = (id == 3) ? "dirin" : "dirout";
+	return 0;
+}
+
+static int ge_parse_dt(struct platform_device *pdev,
+		       struct bgpio_pdata *pdata,
+		       unsigned long *flags)
+{
+	struct device_node *np = pdev->dev.of_node;
+
+	struct resource *res;
+	struct resource nres[] = {
+		DEFINE_RES_MEM_NAMED(0, 1, "dat"),
+		DEFINE_RES_MEM_NAMED(0, 1, "set"),
+		DEFINE_RES_MEM_NAMED(0, 1, "dirin"),
+	};
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res || resource_size(res) != 0x24)
+		return -EINVAL;
+
+	nres[0].start = res->start + 0x04;
+	nres[0].end = nres[0].start + 3;
+	nres[1].start = res->start + 0x08;
+	nres[1].end = nres[1].start + 3;
+	nres[2].start = res->start + 0x00;
+	nres[2].end = nres[2].start + 3;
+	*flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
+	pdata->label = devm_kstrdup(&pdev->dev, np->full_name, GFP_KERNEL);
+	if (!pdata->label)
+		return -ENOMEM;
+
+	if (of_device_is_compatible(np, "ge,imp3a-gpio"))
+		pdata->ngpio = 16;
+	else if (of_device_is_compatible(np, "gef,sbc310-gpio"))
+		pdata->ngpio = 6;
+	else if (of_device_is_compatible(np, "gef,sbc610-gpio"))
+		pdata->ngpio = 19;
+
+	return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
+}
+
+static int moxart_parse_dt(struct platform_device *pdev,
+			   struct bgpio_pdata *pdata,
+			   unsigned long *flags)
+{
+	struct resource *res;
+	struct resource nres[] = {
+		DEFINE_RES_MEM_NAMED(0, 1, "dat"),
+		DEFINE_RES_MEM_NAMED(0, 1, "set"),
+		DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
+	};
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res || resource_size(res) != 0xc)
+		return -EINVAL;
+
+	pdata->label = "moxart-gpio";
+	*flags |= BGPIOF_READ_OUTPUT_REG_SET;
+	nres[0].start = res->start + 0x04;
+	nres[0].end = nres[0].start + 3;
+	nres[1].start = res->start + 0x00;
+	nres[1].end = nres[1].start + 3;
+	nres[2].start = res->start + 0x08;
+	nres[2].end = nres[2].start + 3;
+	return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
+}
+
+static int ts4800_parse_dt(struct platform_device *pdev,
+			   struct bgpio_pdata *pdata,
+			   unsigned long *flags)
+{
+	int err;
+	struct resource *res;
+	struct resource nres[] = {
+		DEFINE_RES_MEM_NAMED(0, 1, "dat"),
+		DEFINE_RES_MEM_NAMED(0, 1, "set"),
+		DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
+	};
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res || resource_size(res) != 0x6)
+		return -EINVAL;
+
+	nres[0].start = res->start;
+	nres[0].end = nres[0].start + 1;
+	nres[1].start = res->start + 0x02;
+	nres[1].end = nres[1].start + 1;
+	nres[2].start = res->start + 0x04;
+	nres[2].end = nres[2].start + 1;
+
+	err = of_property_read_u32(pdev->dev.of_node, "ngpios", &pdata->ngpio);
+	if (err == -EINVAL)
+		pdata->ngpio = 16;
+	else if (err)
+		return err;
+
+	return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
+}
+
 #define ADD(_name, _func) { .compatible = _name, .data = _func }
 
 static const struct of_device_id bgpio_of_match[] = {
 	ADD("linux,gpio-mmio", bgpio_basic_mmio_parse_dt),
+	ADD("cirrus,clps711x-gpio", clps711x_parse_dt),
+	ADD("ge,imp3a-gpio", ge_parse_dt),
+	ADD("gef,sbc310-gpio", ge_parse_dt),
+	ADD("gef,sbc610-gpio", ge_parse_dt),
+	ADD("moxa,moxart-gpio", moxart_parse_dt),
+	ADD("technologic,ts4800-gpio", ts4800_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: [RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Andy Shevchenko]

On Fri, Apr 29, 2016 at 3:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> This patch integrates these GPIO drivers into gpio-generic.
>

>
> +static int clps711x_parse_dt(struct platform_device *pdev,
> +                            struct bgpio_pdata *pdata,
> +                            unsigned long *flags)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
> +       struct resource *res;
> +
> +       if ((id < 0) || (id > 4))
> +               return -ENODEV;
> +
> +       /* PORTE is 3 lines only */
> +       if (id == 4)
> +               pdata->ngpio = 3;
> +
> +       pdata->base = id * 8;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -EINVAL;
> +
> +       res->name = "dat";
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +       if (!res)
> +               return -EINVAL;
> +
> +       /* PORTD is inverted logic for direction register */
> +       res->name = (id == 3) ? "dirin" : "dirout";
> +       return 0;
> +}
> +

I think the three below have enough in common to reorganize data
structure and use one parse function.

So, instead of providing just parsing function, something like

ngpios (0 — read from property)
resource size
start, end pairs for dat/set/dirin.
flags

Have no idea if it's possible to re-use this approach for case above.

> +static int ge_parse_dt(struct platform_device *pdev,
> +                      struct bgpio_pdata *pdata,
> +                      unsigned long *flags)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +
> +       struct resource *res;
> +       struct resource nres[] = {
> +               DEFINE_RES_MEM_NAMED(0, 1, "dat"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "set"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "dirin"),
> +       };
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res || resource_size(res) != 0x24)
> +               return -EINVAL;
> +
> +       nres[0].start = res->start + 0x04;
> +       nres[0].end = nres[0].start + 3;
> +       nres[1].start = res->start + 0x08;
> +       nres[1].end = nres[1].start + 3;
> +       nres[2].start = res->start + 0x00;
> +       nres[2].end = nres[2].start + 3;
> +       *flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
> +       pdata->label = devm_kstrdup(&pdev->dev, np->full_name, GFP_KERNEL);
> +       if (!pdata->label)
> +               return -ENOMEM;
> +
> +       if (of_device_is_compatible(np, "ge,imp3a-gpio"))
> +               pdata->ngpio = 16;
> +       else if (of_device_is_compatible(np, "gef,sbc310-gpio"))
> +               pdata->ngpio = 6;
> +       else if (of_device_is_compatible(np, "gef,sbc610-gpio"))
> +               pdata->ngpio = 19;
> +
> +       return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
> +}
> +
> +static int moxart_parse_dt(struct platform_device *pdev,
> +                          struct bgpio_pdata *pdata,
> +                          unsigned long *flags)
> +{
> +       struct resource *res;
> +       struct resource nres[] = {
> +               DEFINE_RES_MEM_NAMED(0, 1, "dat"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "set"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
> +       };
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res || resource_size(res) != 0xc)
> +               return -EINVAL;
> +
> +       pdata->label = "moxart-gpio";
> +       *flags |= BGPIOF_READ_OUTPUT_REG_SET;
> +       nres[0].start = res->start + 0x04;
> +       nres[0].end = nres[0].start + 3;
> +       nres[1].start = res->start + 0x00;
> +       nres[1].end = nres[1].start + 3;
> +       nres[2].start = res->start + 0x08;
> +       nres[2].end = nres[2].start + 3;
> +       return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
> +}
> +
> +static int ts4800_parse_dt(struct platform_device *pdev,
> +                          struct bgpio_pdata *pdata,
> +                          unsigned long *flags)
> +{
> +       int err;
> +       struct resource *res;
> +       struct resource nres[] = {
> +               DEFINE_RES_MEM_NAMED(0, 1, "dat"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "set"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
> +       };
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res || resource_size(res) != 0x6)
> +               return -EINVAL;
> +
> +       nres[0].start = res->start;
> +       nres[0].end = nres[0].start + 1;
> +       nres[1].start = res->start + 0x02;
> +       nres[1].end = nres[1].start + 1;
> +       nres[2].start = res->start + 0x04;
> +       nres[2].end = nres[2].start + 1;
> +
> +       err = of_property_read_u32(pdev->dev.of_node, "ngpios", &pdata->ngpio);
> +       if (err == -EINVAL)
> +               pdata->ngpio = 16;
> +       else if (err)
> +               return err;
> +
> +       return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
> +}
> +
>  #define ADD(_name, _func) { .compatible = _name, .data = _func }
>
>  static const struct of_device_id bgpio_of_match[] = {
>         ADD("linux,gpio-mmio", bgpio_basic_mmio_parse_dt),
> +       ADD("cirrus,clps711x-gpio", clps711x_parse_dt),
> +       ADD("ge,imp3a-gpio", ge_parse_dt),
> +       ADD("gef,sbc310-gpio", ge_parse_dt),
> +       ADD("gef,sbc610-gpio", ge_parse_dt),
> +       ADD("moxa,moxart-gpio", moxart_parse_dt),
> +       ADD("technologic,ts4800-gpio", ts4800_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
>



-- 
With Best Regards,
Andy Shevchenko

[RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Andy Shevchenko]

On Fri, Apr 29, 2016 at 3:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> This patch integrates these GPIO drivers into gpio-generic.
>

>
> +static int clps711x_parse_dt(struct platform_device *pdev,
> +                            struct bgpio_pdata *pdata,
> +                            unsigned long *flags)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       int id = np ? of_alias_get_id(np, "gpio") : pdev->id;
> +       struct resource *res;
> +
> +       if ((id < 0) || (id > 4))
> +               return -ENODEV;
> +
> +       /* PORTE is 3 lines only */
> +       if (id == 4)
> +               pdata->ngpio = 3;
> +
> +       pdata->base = id * 8;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -EINVAL;
> +
> +       res->name = "dat";
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +       if (!res)
> +               return -EINVAL;
> +
> +       /* PORTD is inverted logic for direction register */
> +       res->name = (id == 3) ? "dirin" : "dirout";
> +       return 0;
> +}
> +

I think the three below have enough in common to reorganize data
structure and use one parse function.

So, instead of providing just parsing function, something like

ngpios (0 ? read from property)
resource size
start, end pairs for dat/set/dirin.
flags

Have no idea if it's possible to re-use this approach for case above.

> +static int ge_parse_dt(struct platform_device *pdev,
> +                      struct bgpio_pdata *pdata,
> +                      unsigned long *flags)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +
> +       struct resource *res;
> +       struct resource nres[] = {
> +               DEFINE_RES_MEM_NAMED(0, 1, "dat"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "set"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "dirin"),
> +       };
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res || resource_size(res) != 0x24)
> +               return -EINVAL;
> +
> +       nres[0].start = res->start + 0x04;
> +       nres[0].end = nres[0].start + 3;
> +       nres[1].start = res->start + 0x08;
> +       nres[1].end = nres[1].start + 3;
> +       nres[2].start = res->start + 0x00;
> +       nres[2].end = nres[2].start + 3;
> +       *flags |= BGPIOF_BIG_ENDIAN_BYTE_ORDER;
> +       pdata->label = devm_kstrdup(&pdev->dev, np->full_name, GFP_KERNEL);
> +       if (!pdata->label)
> +               return -ENOMEM;
> +
> +       if (of_device_is_compatible(np, "ge,imp3a-gpio"))
> +               pdata->ngpio = 16;
> +       else if (of_device_is_compatible(np, "gef,sbc310-gpio"))
> +               pdata->ngpio = 6;
> +       else if (of_device_is_compatible(np, "gef,sbc610-gpio"))
> +               pdata->ngpio = 19;
> +
> +       return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
> +}
> +
> +static int moxart_parse_dt(struct platform_device *pdev,
> +                          struct bgpio_pdata *pdata,
> +                          unsigned long *flags)
> +{
> +       struct resource *res;
> +       struct resource nres[] = {
> +               DEFINE_RES_MEM_NAMED(0, 1, "dat"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "set"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
> +       };
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res || resource_size(res) != 0xc)
> +               return -EINVAL;
> +
> +       pdata->label = "moxart-gpio";
> +       *flags |= BGPIOF_READ_OUTPUT_REG_SET;
> +       nres[0].start = res->start + 0x04;
> +       nres[0].end = nres[0].start + 3;
> +       nres[1].start = res->start + 0x00;
> +       nres[1].end = nres[1].start + 3;
> +       nres[2].start = res->start + 0x08;
> +       nres[2].end = nres[2].start + 3;
> +       return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
> +}
> +
> +static int ts4800_parse_dt(struct platform_device *pdev,
> +                          struct bgpio_pdata *pdata,
> +                          unsigned long *flags)
> +{
> +       int err;
> +       struct resource *res;
> +       struct resource nres[] = {
> +               DEFINE_RES_MEM_NAMED(0, 1, "dat"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "set"),
> +               DEFINE_RES_MEM_NAMED(0, 1, "dirout"),
> +       };
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res || resource_size(res) != 0x6)
> +               return -EINVAL;
> +
> +       nres[0].start = res->start;
> +       nres[0].end = nres[0].start + 1;
> +       nres[1].start = res->start + 0x02;
> +       nres[1].end = nres[1].start + 1;
> +       nres[2].start = res->start + 0x04;
> +       nres[2].end = nres[2].start + 1;
> +
> +       err = of_property_read_u32(pdev->dev.of_node, "ngpios", &pdata->ngpio);
> +       if (err == -EINVAL)
> +               pdata->ngpio = 16;
> +       else if (err)
> +               return err;
> +
> +       return platform_device_add_resources(pdev, nres, ARRAY_SIZE(nres));
> +}
> +
>  #define ADD(_name, _func) { .compatible = _name, .data = _func }
>
>  static const struct of_device_id bgpio_of_match[] = {
>         ADD("linux,gpio-mmio", bgpio_basic_mmio_parse_dt),
> +       ADD("cirrus,clps711x-gpio", clps711x_parse_dt),
> +       ADD("ge,imp3a-gpio", ge_parse_dt),
> +       ADD("gef,sbc310-gpio", ge_parse_dt),
> +       ADD("gef,sbc610-gpio", ge_parse_dt),
> +       ADD("moxa,moxart-gpio", moxart_parse_dt),
> +       ADD("technologic,ts4800-gpio", ts4800_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
>



-- 
With Best Regards,
Andy Shevchenko

Re: [RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> This patch integrates these GPIO drivers into gpio-generic.
>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

Very elegant. Let's wait for some feedback/ACKs from
the driver maintainers, but this looks very nice.

Do you want to address Andy's optimization suggestion directly
or as a follow-on patch?

Yours,
Linus Walleij

[RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> This patch integrates these GPIO drivers into gpio-generic.
>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

Very elegant. Let's wait for some feedback/ACKs from
the driver maintainers, but this looks very nice.

Do you want to address Andy's optimization suggestion directly
or as a follow-on patch?

Yours,
Linus Walleij

Re: [RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Christian Lamparter]

On Friday, April 29, 2016 04:18:20 PM Linus Walleij wrote:
> On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> 
> > This patch integrates these GPIO drivers into gpio-generic.
> >
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> 
> Very elegant. Let's wait for some feedback/ACKs from
> the driver maintainers, but this looks very nice.
Oh by the way: who's maintaining them? get_maintainer.pl just
lists the usual gpio people. I can try to add the module authors.
However, I don't know if these are working or dead. I ran into an
issue once (for something else) that I got the dreaded "delivery
notification failed" messages, but I can't rebroadcast them on the
ML (because that would be very silly).

Anyway, I'll rebase my patch series (without the two patches you
had already merged)

> Do you want to address Andy's optimization suggestion directly
> or as a follow-on patch?
Oh, this is a RFC. I didn't expect anything to be merged just yet.
Of course, I'll try to integrate his suggestion. I'll do that once
I reply to Rob/Mark/You about the "linux,gpio-mmio",
"memory-mapped-gpio" situation etc.

Regards,
Christian Lamparter

[RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Christian Lamparter]

On Friday, April 29, 2016 04:18:20 PM Linus Walleij wrote:
> On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
> <chunkeey@googlemail.com> wrote:
> 
> > This patch integrates these GPIO drivers into gpio-generic.
> >
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> 
> Very elegant. Let's wait for some feedback/ACKs from
> the driver maintainers, but this looks very nice.
Oh by the way: who's maintaining them? get_maintainer.pl just
lists the usual gpio people. I can try to add the module authors.
However, I don't know if these are working or dead. I ran into an
issue once (for something else) that I got the dreaded "delivery
notification failed" messages, but I can't rebroadcast them on the
ML (because that would be very silly).

Anyway, I'll rebase my patch series (without the two patches you
had already merged)

> Do you want to address Andy's optimization suggestion directly
> or as a follow-on patch?
Oh, this is a RFC. I didn't expect anything to be merged just yet.
Of course, I'll try to integrate his suggestion. I'll do that once
I reply to Rob/Mark/You about the "linux,gpio-mmio",
"memory-mapped-gpio" situation etc.

Regards,
Christian Lamparter

Re: [RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Linus Walleij]

On Fri, Apr 29, 2016 at 9:06 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Friday, April 29, 2016 04:18:20 PM Linus Walleij wrote:

>> Very elegant. Let's wait for some feedback/ACKs from
>> the driver maintainers, but this looks very nice.

> Oh by the way: who's maintaining them? get_maintainer.pl just
> lists the usual gpio people.

I usually just git log drivers/gpio/gpio-foo.c
and see what turns up, then sift out to avoid any generic
maintenance patches and mail the remaining people who
made real functional changes and seem  to be using the
real hardware.

> Anyway, I'll rebase my patch series (without the two patches you
> had already merged)

Awesome.

>> Do you want to address Andy's optimization suggestion directly
>> or as a follow-on patch?
>
> Oh, this is a RFC. I didn't expect anything to be merged just yet.
> Of course, I'll try to integrate his suggestion. I'll do that once
> I reply to Rob/Mark/You about the "linux,gpio-mmio",
> "memory-mapped-gpio" situation etc.

True, OK no hurries, I just liked the patches a lot.

Yours,
Linus Walleij

[RFC v5 3/4] gpio: move clps711x, moxart, ts4800 and gpio-ge into gpio-generic

[Linus Walleij]

On Fri, Apr 29, 2016 at 9:06 PM, Christian Lamparter
<chunkeey@googlemail.com> wrote:
> On Friday, April 29, 2016 04:18:20 PM Linus Walleij wrote:

>> Very elegant. Let's wait for some feedback/ACKs from
>> the driver maintainers, but this looks very nice.

> Oh by the way: who's maintaining them? get_maintainer.pl just
> lists the usual gpio people.

I usually just git log drivers/gpio/gpio-foo.c
and see what turns up, then sift out to avoid any generic
maintenance patches and mail the remaining people who
made real functional changes and seem  to be using the
real hardware.

> Anyway, I'll rebase my patch series (without the two patches you
> had already merged)

Awesome.

>> Do you want to address Andy's optimization suggestion directly
>> or as a follow-on patch?
>
> Oh, this is a RFC. I didn't expect anything to be merged just yet.
> Of course, I'll try to integrate his suggestion. I'll do that once
> I reply to Rob/Mark/You about the "linux,gpio-mmio",
> "memory-mapped-gpio" situation etc.

True, OK no hurries, I just liked the patches a lot.

Yours,
Linus Walleij

[RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Christian Lamparter]

From: Álvaro Fernández Rojas <noltari@gmail.com>

This patch adds the device tree bindings for the gpio-mmio.
The gpio-mmio is already part of a the GPIO generic library.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
new file mode 100644
index 0000000..cc7f0b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
@@ -0,0 +1,73 @@
+Bindings for the generic driver for memory-mapped GPIO controllers.
+
+Required properties:
+	- compatible: should be "linux,gpio-mmio"
+	- reg-names: must contain
+		"dat" - data register
+		may contain
+		"set" - data set register
+		"clr" - data clear register
+		"dirout" - direction output register
+		"dirin" - direction input 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
+	- gpio-controller: Marks the device node as a gpio controller.
+
+Optional properties:
+	- ngpio: specifies the number of gpio mapped in the register.
+	- big-endian: force big endian register accesses.
+	- big-endian-byte-order: assign GPIOs in reverse order.
+	- unreadable-reg-set: data set register is not readable.
+	- read-output-reg-set: cache value set for reads.
+	- unreadable-reg-dir: dirout/dirin register is not readable.
+	- no-output: GPIOs are read-only.
+
+The GPIO generic library provides support for memory-mapped GPIO
+controllers. The configuration is detected by which resources are present.
+The simplest form of a GPIO controller that the driver support is just a
+single "dat" register, where GPIO state can be read and/or written.
+However, the driver supports far more:
+	- 8/16/32/64 bits registers. The number of GPIOs is automatically
+	  determined by the width of the registers.
+	- GPIO controllers with clear/set registers.
+	- GPIO controllers with a single "dat" register.
+	- Big endian bits/GPIOs ordering.
+
+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").
+
+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.
+
+Examples:
+
+	/* Configuration for single input/output register
+	 * for eight simple bidirectional GPIOs.
+	 */
+	gpio_a_1 {
+		compatible = "linux,gpio-mmio";
+		reg = <0x18000000 0x1>;
+		reg-names = "dat";
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+
+	/* Configuration for set/clear pair registers with
+	 * 32 output direction register GPIOs.
+	 */
+	gpio_b_2 {
+		compatible = "linux,gpio-mmio";
+		reg = <0x18000000 0x4>, <0x18000010 0x4>,
+		      <0x18000004 0x4>, <0x18000008 0x4>;
+		reg-names = "dat", "set", "clr", "dirout";
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
-- 
2.8.1

[RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Christian Lamparter]

From: ?lvaro Fern?ndez Rojas <noltari@gmail.com>

This patch adds the device tree bindings for the gpio-mmio.
The gpio-mmio is already part of a the GPIO generic library.

Signed-off-by: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
---
 .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
new file mode 100644
index 0000000..cc7f0b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
@@ -0,0 +1,73 @@
+Bindings for the generic driver for memory-mapped GPIO controllers.
+
+Required properties:
+	- compatible: should be "linux,gpio-mmio"
+	- reg-names: must contain
+		"dat" - data register
+		may contain
+		"set" - data set register
+		"clr" - data clear register
+		"dirout" - direction output register
+		"dirin" - direction input 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
+	- gpio-controller: Marks the device node as a gpio controller.
+
+Optional properties:
+	- ngpio: specifies the number of gpio mapped in the register.
+	- big-endian: force big endian register accesses.
+	- big-endian-byte-order: assign GPIOs in reverse order.
+	- unreadable-reg-set: data set register is not readable.
+	- read-output-reg-set: cache value set for reads.
+	- unreadable-reg-dir: dirout/dirin register is not readable.
+	- no-output: GPIOs are read-only.
+
+The GPIO generic library provides support for memory-mapped GPIO
+controllers. The configuration is detected by which resources are present.
+The simplest form of a GPIO controller that the driver support is just a
+single "dat" register, where GPIO state can be read and/or written.
+However, the driver supports far more:
+	- 8/16/32/64 bits registers. The number of GPIOs is automatically
+	  determined by the width of the registers.
+	- GPIO controllers with clear/set registers.
+	- GPIO controllers with a single "dat" register.
+	- Big endian bits/GPIOs ordering.
+
+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").
+
+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.
+
+Examples:
+
+	/* Configuration for single input/output register
+	 * for eight simple bidirectional GPIOs.
+	 */
+	gpio_a_1 {
+		compatible = "linux,gpio-mmio";
+		reg = <0x18000000 0x1>;
+		reg-names = "dat";
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+
+	/* Configuration for set/clear pair registers with
+	 * 32 output direction register GPIOs.
+	 */
+	gpio_b_2 {
+		compatible = "linux,gpio-mmio";
+		reg = <0x18000000 0x4>, <0x18000010 0x4>,
+		      <0x18000004 0x4>, <0x18000008 0x4>;
+		reg-names = "dat", "set", "clr", "dirout";
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
-- 
2.8.1

Re: [RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Mark Rutland]

Hi,

As a general thing, please put the binding earlier in a series than code
implemeting it, as that that makes it easier to review the series
in-order (with context from the binding making code review easier). See
Documentation/devicetree/bindings/submitting-patches.txt

On Fri, Apr 29, 2016 at 02:53:17AM +0200, Christian Lamparter wrote:
> From: Álvaro Fernández Rojas <noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This patch adds the device tree bindings for the gpio-mmio.
> The gpio-mmio is already part of a the GPIO generic library.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Christian Lamparter <chunkeey-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
>  .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> new file mode 100644
> index 0000000..cc7f0b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> @@ -0,0 +1,73 @@
> +Bindings for the generic driver for memory-mapped GPIO controllers.
> +

Bindings should be for hardware (either specific device models, or for
classes), and not for Linux drivers. The latter is subject to arbitrary
changes while the former is not, as old hardware continues to exist and
does not change while drivers get completely reworked.

So please frame this binding document in terms of the class of hardware
you are trying to address. Please provide some introduction to the
assumptions that you are making about said hardware, such that it's
obvious when one needs a more specific binding.

I share the same fears that Rob mentioned in that while this may appear
simple now, the limitations are unclear, and this effectively prevents
us from accurately/specifically describing some hardware.

Regardless, I've taken the time to review the below, and I have a number
of comments.

> +Required properties:
> +	- compatible: should be "linux,gpio-mmio"

Even with the above comments, this binding name would be valid.

> +	- reg-names: must contain
> +		"dat" - data register
> +		may contain
> +		"set" - data set register
> +		"clr" - data clear register
> +		"dirout" - direction output register
> +		"dirin" - direction input register

Below, we mention endianness. Are registers an arbitrary number of bytes
long? In what unit size are they grouped (e.g. {8,16,32,64}-bit)?

What are the expectations for the set/clear/dirin/dirout registers? Are
they write-one to modify, or write-zero to modify?

For the names, don't bother abbreviating (i.e. use "data", "set",
"clear").

These names might clash with the real names a specific HW model applies
to its register sets, which means that this binding is exclusive w.r.t.
a specific binding for a device (i.e. it cannot be extended with
device-specific information).

If we're happy with that, then we must call out the expected limitations
on the use of this binding, or we end up with the not-so-simple-any-more
issues Rob mentioned previously. 

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

Where the cells encode what? I'm guessing this is <$idx $flags>, but you
should spell that out explicitly.

> +	- gpio-controller: Marks the device node as a gpio controller.
> +
> +Optional properties:
> +	- ngpio: specifies the number of gpio mapped in the register.
> +	- big-endian: force big endian register accesses.
> +	- big-endian-byte-order: assign GPIOs in reverse order.

I cannot parse this last description. It needs a better wording.

I guess this means that the indices (in the first GPIO cell) are applied
in descending order for ascending chunks of the registers (and that is a
poor description, too).

> +	- unreadable-reg-set: data set register is not readable.
> +	- read-output-reg-set: cache value set for reads.
> +	- unreadable-reg-dir: dirout/dirin register is not readable.
> +	- no-output: GPIOs are read-only.
> +
> +The GPIO generic library provides support for memory-mapped GPIO
> +controllers. The configuration is detected by which resources are present.
> +The simplest form of a GPIO controller that the driver support is just a
> +single "dat" register, where GPIO state can be read and/or written.
> +However, the driver supports far more:
> +	- 8/16/32/64 bits registers. The number of GPIOs is automatically
> +	  determined by the width of the registers.
> +	- GPIO controllers with clear/set registers.
> +	- GPIO controllers with a single "dat" register.
> +	- Big endian bits/GPIOs ordering.

Reword this in terms of the class of hardware you are trying to support,
(rather than the specific library code you are using), and move it to
the introduction at the top of the binding.

Thanks,
Mark.

> +
> +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").
> +
> +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.
> +
> +Examples:
> +
> +	/* Configuration for single input/output register
> +	 * for eight simple bidirectional GPIOs.
> +	 */
> +	gpio_a_1 {
> +		compatible = "linux,gpio-mmio";
> +		reg = <0x18000000 0x1>;
> +		reg-names = "dat";
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> +
> +	/* Configuration for set/clear pair registers with
> +	 * 32 output direction register GPIOs.
> +	 */
> +	gpio_b_2 {
> +		compatible = "linux,gpio-mmio";
> +		reg = <0x18000000 0x4>, <0x18000010 0x4>,
> +		      <0x18000004 0x4>, <0x18000008 0x4>;
> +		reg-names = "dat", "set", "clr", "dirout";
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> -- 
> 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

[RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Mark Rutland]

Hi,

As a general thing, please put the binding earlier in a series than code
implemeting it, as that that makes it easier to review the series
in-order (with context from the binding making code review easier). See
Documentation/devicetree/bindings/submitting-patches.txt

On Fri, Apr 29, 2016 at 02:53:17AM +0200, Christian Lamparter wrote:
> From: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
> 
> This patch adds the device tree bindings for the gpio-mmio.
> The gpio-mmio is already part of a the GPIO generic library.
> 
> Signed-off-by: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
>  .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> new file mode 100644
> index 0000000..cc7f0b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> @@ -0,0 +1,73 @@
> +Bindings for the generic driver for memory-mapped GPIO controllers.
> +

Bindings should be for hardware (either specific device models, or for
classes), and not for Linux drivers. The latter is subject to arbitrary
changes while the former is not, as old hardware continues to exist and
does not change while drivers get completely reworked.

So please frame this binding document in terms of the class of hardware
you are trying to address. Please provide some introduction to the
assumptions that you are making about said hardware, such that it's
obvious when one needs a more specific binding.

I share the same fears that Rob mentioned in that while this may appear
simple now, the limitations are unclear, and this effectively prevents
us from accurately/specifically describing some hardware.

Regardless, I've taken the time to review the below, and I have a number
of comments.

> +Required properties:
> +	- compatible: should be "linux,gpio-mmio"

Even with the above comments, this binding name would be valid.

> +	- reg-names: must contain
> +		"dat" - data register
> +		may contain
> +		"set" - data set register
> +		"clr" - data clear register
> +		"dirout" - direction output register
> +		"dirin" - direction input register

Below, we mention endianness. Are registers an arbitrary number of bytes
long? In what unit size are they grouped (e.g. {8,16,32,64}-bit)?

What are the expectations for the set/clear/dirin/dirout registers? Are
they write-one to modify, or write-zero to modify?

For the names, don't bother abbreviating (i.e. use "data", "set",
"clear").

These names might clash with the real names a specific HW model applies
to its register sets, which means that this binding is exclusive w.r.t.
a specific binding for a device (i.e. it cannot be extended with
device-specific information).

If we're happy with that, then we must call out the expected limitations
on the use of this binding, or we end up with the not-so-simple-any-more
issues Rob mentioned previously. 

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

Where the cells encode what? I'm guessing this is <$idx $flags>, but you
should spell that out explicitly.

> +	- gpio-controller: Marks the device node as a gpio controller.
> +
> +Optional properties:
> +	- ngpio: specifies the number of gpio mapped in the register.
> +	- big-endian: force big endian register accesses.
> +	- big-endian-byte-order: assign GPIOs in reverse order.

I cannot parse this last description. It needs a better wording.

I guess this means that the indices (in the first GPIO cell) are applied
in descending order for ascending chunks of the registers (and that is a
poor description, too).

> +	- unreadable-reg-set: data set register is not readable.
> +	- read-output-reg-set: cache value set for reads.
> +	- unreadable-reg-dir: dirout/dirin register is not readable.
> +	- no-output: GPIOs are read-only.
> +
> +The GPIO generic library provides support for memory-mapped GPIO
> +controllers. The configuration is detected by which resources are present.
> +The simplest form of a GPIO controller that the driver support is just a
> +single "dat" register, where GPIO state can be read and/or written.
> +However, the driver supports far more:
> +	- 8/16/32/64 bits registers. The number of GPIOs is automatically
> +	  determined by the width of the registers.
> +	- GPIO controllers with clear/set registers.
> +	- GPIO controllers with a single "dat" register.
> +	- Big endian bits/GPIOs ordering.

Reword this in terms of the class of hardware you are trying to support,
(rather than the specific library code you are using), and move it to
the introduction at the top of the binding.

Thanks,
Mark.

> +
> +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").
> +
> +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.
> +
> +Examples:
> +
> +	/* Configuration for single input/output register
> +	 * for eight simple bidirectional GPIOs.
> +	 */
> +	gpio_a_1 {
> +		compatible = "linux,gpio-mmio";
> +		reg = <0x18000000 0x1>;
> +		reg-names = "dat";
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> +
> +	/* Configuration for set/clear pair registers with
> +	 * 32 output direction register GPIOs.
> +	 */
> +	gpio_b_2 {
> +		compatible = "linux,gpio-mmio";
> +		reg = <0x18000000 0x4>, <0x18000010 0x4>,
> +		      <0x18000004 0x4>, <0x18000008 0x4>;
> +		reg-names = "dat", "set", "clr", "dirout";
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> -- 
> 2.8.1
> 

Re: [RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Mark Rutland]

Hi,

As a general thing, please put the binding earlier in a series than code
implemeting it, as that that makes it easier to review the series
in-order (with context from the binding making code review easier). See
Documentation/devicetree/bindings/submitting-patches.txt

On Fri, Apr 29, 2016 at 02:53:17AM +0200, Christian Lamparter wrote:
> From: Álvaro Fernández Rojas <noltari@gmail.com>
> 
> This patch adds the device tree bindings for the gpio-mmio.
> The gpio-mmio is already part of a the GPIO generic library.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> ---
>  .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> new file mode 100644
> index 0000000..cc7f0b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> @@ -0,0 +1,73 @@
> +Bindings for the generic driver for memory-mapped GPIO controllers.
> +

Bindings should be for hardware (either specific device models, or for
classes), and not for Linux drivers. The latter is subject to arbitrary
changes while the former is not, as old hardware continues to exist and
does not change while drivers get completely reworked.

So please frame this binding document in terms of the class of hardware
you are trying to address. Please provide some introduction to the
assumptions that you are making about said hardware, such that it's
obvious when one needs a more specific binding.

I share the same fears that Rob mentioned in that while this may appear
simple now, the limitations are unclear, and this effectively prevents
us from accurately/specifically describing some hardware.

Regardless, I've taken the time to review the below, and I have a number
of comments.

> +Required properties:
> +	- compatible: should be "linux,gpio-mmio"

Even with the above comments, this binding name would be valid.

> +	- reg-names: must contain
> +		"dat" - data register
> +		may contain
> +		"set" - data set register
> +		"clr" - data clear register
> +		"dirout" - direction output register
> +		"dirin" - direction input register

Below, we mention endianness. Are registers an arbitrary number of bytes
long? In what unit size are they grouped (e.g. {8,16,32,64}-bit)?

What are the expectations for the set/clear/dirin/dirout registers? Are
they write-one to modify, or write-zero to modify?

For the names, don't bother abbreviating (i.e. use "data", "set",
"clear").

These names might clash with the real names a specific HW model applies
to its register sets, which means that this binding is exclusive w.r.t.
a specific binding for a device (i.e. it cannot be extended with
device-specific information).

If we're happy with that, then we must call out the expected limitations
on the use of this binding, or we end up with the not-so-simple-any-more
issues Rob mentioned previously. 

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

Where the cells encode what? I'm guessing this is <$idx $flags>, but you
should spell that out explicitly.

> +	- gpio-controller: Marks the device node as a gpio controller.
> +
> +Optional properties:
> +	- ngpio: specifies the number of gpio mapped in the register.
> +	- big-endian: force big endian register accesses.
> +	- big-endian-byte-order: assign GPIOs in reverse order.

I cannot parse this last description. It needs a better wording.

I guess this means that the indices (in the first GPIO cell) are applied
in descending order for ascending chunks of the registers (and that is a
poor description, too).

> +	- unreadable-reg-set: data set register is not readable.
> +	- read-output-reg-set: cache value set for reads.
> +	- unreadable-reg-dir: dirout/dirin register is not readable.
> +	- no-output: GPIOs are read-only.
> +
> +The GPIO generic library provides support for memory-mapped GPIO
> +controllers. The configuration is detected by which resources are present.
> +The simplest form of a GPIO controller that the driver support is just a
> +single "dat" register, where GPIO state can be read and/or written.
> +However, the driver supports far more:
> +	- 8/16/32/64 bits registers. The number of GPIOs is automatically
> +	  determined by the width of the registers.
> +	- GPIO controllers with clear/set registers.
> +	- GPIO controllers with a single "dat" register.
> +	- Big endian bits/GPIOs ordering.

Reword this in terms of the class of hardware you are trying to support,
(rather than the specific library code you are using), and move it to
the introduction at the top of the binding.

Thanks,
Mark.

> +
> +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").
> +
> +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.
> +
> +Examples:
> +
> +	/* Configuration for single input/output register
> +	 * for eight simple bidirectional GPIOs.
> +	 */
> +	gpio_a_1 {
> +		compatible = "linux,gpio-mmio";
> +		reg = <0x18000000 0x1>;
> +		reg-names = "dat";
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> +
> +	/* Configuration for set/clear pair registers with
> +	 * 32 output direction register GPIOs.
> +	 */
> +	gpio_b_2 {
> +		compatible = "linux,gpio-mmio";
> +		reg = <0x18000000 0x4>, <0x18000010 0x4>,
> +		      <0x18000004 0x4>, <0x18000008 0x4>;
> +		reg-names = "dat", "set", "clr", "dirout";
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +	};
> -- 
> 2.8.1
> 

Re: [RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Christian Lamparter]

Hello,

(I'm sort of answering both of yours and Linus' questions)

On Friday, April 29, 2016 12:15:01 PM Mark Rutland wrote:
> As a general thing, please put the binding earlier in a series than code
> implemeting it, as that that makes it easier to review the series
> in-order (with context from the binding making code review easier). See
> Documentation/devicetree/bindings/submitting-patches.txt

Understood. I have to rebase my series anyway since Linus Walleij already
merged two related patches (the bugfix and the rename patch). For the new
series I'll put the binding on the top.
 
> On Fri, Apr 29, 2016 at 02:53:17AM +0200, Christian Lamparter wrote:
> > From: Álvaro Fernández Rojas <noltari@gmail.com>
> > 
> > This patch adds the device tree bindings for the gpio-mmio.
> > The gpio-mmio is already part of a the GPIO generic library.
> > 
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> > ---
> >  .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > new file mode 100644
> > index 0000000..cc7f0b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > @@ -0,0 +1,73 @@
> > +Bindings for the generic driver for memory-mapped GPIO controllers.
> > +
> 
> Bindings should be for hardware (either specific device models, or for
> classes), and not for Linux drivers. The latter is subject to arbitrary
> changes while the former is not, as old hardware continues to exist and
> does not change while drivers get completely reworked.
Ok, maybe here's part of the problem. I'm thinking that the GPIOs devices
I'm referring to:
	MyBook Live
	brcm63xx (which is a popular router SoC so there are many boards)

are the same common class: "memory-mapped gpio". It's is a bit
unfortunate that the linux driver file name was gpio-generic.c. (But
this since been fixed... actually funny thing is that it started as:
basic_mmio_gpio.c)

The important bit is that, when I looked around: there is was already
an existing device binding in the form of a platform_device called:
"basic-mmio-gpio" [0].

commit aeec56e331c6d2750de02ef34b305338305ca690
Author: Anton Vorontsov <cbouatmailru@gmail.com>
Date:   Wed Oct 27 15:33:15 2010 -0700

    gpio: add driver for basic memory-mapped GPIO controllers
    
    The basic GPIO controllers may be 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.
    
    These controllers may not provide any means of pin setup
    (in/out/open drain).
    
    The driver supports:
    - 8/16/32/64 bits registers;
    - GPIO controllers with clear/set registers;
    - GPIO controllers with a single "data" register;
    - Big endian bits/GPIOs ordering (mostly used on PowerPC).

And there are a users of this "basic-mmio-gpio" interface:
arch/arm/mach-s3c64xx/mach-crag6410.c
arch/arm/mach-omap1/board-ams-delta.c
arch/arm/mach-imx/mach-mx21ads.c
arch/arm/mach-clps711x/board-p720t.c
arch/arm/mach-clps711x/board-autcpu12.c

to that end: The patch series proposed by Mr. Fernández simply added a
device-tree-parser for translating the fdt into a bgpio_pdata platform
structure and use the existing interface with the same "basic-mmio-gpio"
compatible string. So unless steps are taken to remove or rename this
"basic-mmio-gpio" interface there will be patches like this (in your inboxes).
[I know: It's a silly idea to actually touch it, as it'll break compatibility]

> So please frame this binding document in terms of the class of hardware
> you are trying to address. Please provide some introduction to the
> assumptions that you are making about said hardware, such that it's
> obvious when one needs a more specific binding.
> 
> I share the same fears that Rob mentioned in that while this may appear
> simple now, the limitations are unclear, and this effectively prevents
> us from accurately/specifically describing some hardware.
Well, I think this has been trainwrecked enough. But there's a way out:
I would really like to preserve the bgpio_basic_mmio_parse_dt so probably
going to introduce a "wd,mbl-gpio" binding for my own hardware and let 
potential new user be able to reuse the generic dt parser. This will cut
down on the work required to add a device. As they will only need to add
an compatible entry to of_device_id list of the driver like this:
	"brcm,bcm63xx-gpio", bgpio_basic_mmio_parse_dt

Will this work for you Linus, Rob and Mark?

That said, I'm still going through the rest of the post. As some of the
issues are still relevant for the "wd,mbl-gpio" as well.

> Regardless, I've taken the time to review the below, and I have a number
> of comments.
> 
> > +Required properties:
> > +	- reg-names: must contain
> > +		"dat" - data register
> > +		may contain
> > +		"set" - data set register
> > +		"clr" - data clear register
> > +		"dirout" - direction output register
> > +		"dirin" - direction input register
> 
> Below, we mention endianness. Are registers an arbitrary number of bytes
> long? In what unit size are they grouped (e.g. {8,16,32,64}-bit)?
No, sadly they can't be (commit log above). The basic_mmio_gpio
device only supports 8/16/32/64 bits registers and you only can
have one register. 

In a nutshell, the basic_mmio_gpio is a glorified
io(read|write)(8|16|32|64)[be] interface. And that's what makes it appealing.
It won't be long until someone clobbers together gpiohog, gpio-led,
gpio-key-polled, led-triggers definitions and implements a full turing machine
within a fdt just for the fun of it.
 
> What are the expectations for the set/clear/dirin/dirout registers? Are
> they write-one to modify, or write-zero to modify?

There's a paragraph in the driver:
"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."

I'll add that to the binding.

About dirin/dirout. There can only be either be one of the two.
dirin: 1 = GPIO is input, 0 = GPIO is output
dirout: 1 = GPIO is output, 1 = GPIO is input

> For the names, don't bother abbreviating (i.e. use "data", "set",
> "clear").
>
> These names might clash with the real names a specific HW model applies
> to its register sets, which means that this binding is exclusive w.r.t.
> a specific binding for a device (i.e. it cannot be extended with
> device-specific information).
> 
> If we're happy with that, then we must call out the expected limitations
> on the use of this binding, or we end up with the not-so-simple-any-more
> issues Rob mentioned previously. 

Sadly the resource names are part of the original commit. It's not possible
without adding a resource rename step in the dt parser or breaking 
compatibility with existing interface. So, the question would be: which one
do you actually want or better yet: "what can we agree on?".
 
> > +	- 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
> 
> Where the cells encode what? I'm guessing this is <$idx $flags>, but you
> should spell that out explicitly.
Ok. I'll add that.
 
> > +	- gpio-controller: Marks the device node as a gpio controller.
> > +
> > +Optional properties:
> > +	- ngpio: specifies the number of gpio mapped in the register.
> > +	- big-endian: force big endian register accesses.
> > +	- big-endian-byte-order: assign GPIOs in reverse order.
> 
> I cannot parse this last description. It needs a better wording.
> 
> I guess this means that the indices (in the first GPIO cell) are applied
> in descending order for ascending chunks of the registers (and that is a
> poor description, too).

I looked up the proper CS terms: 
"big-endian-byte-order: assign GPIOs in MSB 0 bit numbering order."
(The default is: LSB 0 bit numbering)

> > +	- unreadable-reg-set: data set register is not readable.
> > +	- read-output-reg-set: cache value set for reads.
> > +	- unreadable-reg-dir: dirout/dirin register is not readable.
> > +	- no-output: GPIOs are read-only.
> > +
> > +The GPIO generic library provides support for memory-mapped GPIO
> > +controllers. The configuration is detected by which resources are present.
> > +The simplest form of a GPIO controller that the driver support is just a
> > +single "dat" register, where GPIO state can be read and/or written.
> > +However, the driver supports far more:
> > +	- 8/16/32/64 bits registers. The number of GPIOs is automatically
> > +	  determined by the width of the registers.
> > +	- GPIO controllers with clear/set registers.
> > +	- GPIO controllers with a single "dat" register.
> > +	- Big endian bits/GPIOs ordering.
> 
> Reword this in terms of the class of hardware you are trying to support,
> (rather than the specific library code you are using), and move it to
> the introduction at the top of the binding.

Yes. That said the MyBook Live only has a bidirectional "dat" register
and 8 bit for GPIOs (this simplifies a lot of things).

Unless of course, we can somehow agree on the generic approach.
 
Regards,
Christian

[0] <http://marc.info/?l=git-commits-head&m=128867795112158&w=2>

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

[RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Christian Lamparter]

Hello,

(I'm sort of answering both of yours and Linus' questions)

On Friday, April 29, 2016 12:15:01 PM Mark Rutland wrote:
> As a general thing, please put the binding earlier in a series than code
> implemeting it, as that that makes it easier to review the series
> in-order (with context from the binding making code review easier). See
> Documentation/devicetree/bindings/submitting-patches.txt

Understood. I have to rebase my series anyway since Linus Walleij already
merged two related patches (the bugfix and the rename patch). For the new
series I'll put the binding on the top.
 
> On Fri, Apr 29, 2016 at 02:53:17AM +0200, Christian Lamparter wrote:
> > From: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
> > 
> > This patch adds the device tree bindings for the gpio-mmio.
> > The gpio-mmio is already part of a the GPIO generic library.
> > 
> > Signed-off-by: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> > ---
> >  .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > new file mode 100644
> > index 0000000..cc7f0b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > @@ -0,0 +1,73 @@
> > +Bindings for the generic driver for memory-mapped GPIO controllers.
> > +
> 
> Bindings should be for hardware (either specific device models, or for
> classes), and not for Linux drivers. The latter is subject to arbitrary
> changes while the former is not, as old hardware continues to exist and
> does not change while drivers get completely reworked.
Ok, maybe here's part of the problem. I'm thinking that the GPIOs devices
I'm referring to:
	MyBook Live
	brcm63xx (which is a popular router SoC so there are many boards)

are the same common class: "memory-mapped gpio". It's is a bit
unfortunate that the linux driver file name was gpio-generic.c. (But
this since been fixed... actually funny thing is that it started as:
basic_mmio_gpio.c)

The important bit is that, when I looked around: there is was already
an existing device binding in the form of a platform_device called:
"basic-mmio-gpio" [0].

commit aeec56e331c6d2750de02ef34b305338305ca690
Author: Anton Vorontsov <cbouatmailru@gmail.com>
Date:   Wed Oct 27 15:33:15 2010 -0700

    gpio: add driver for basic memory-mapped GPIO controllers
    
    The basic GPIO controllers may be 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.
    
    These controllers may not provide any means of pin setup
    (in/out/open drain).
    
    The driver supports:
    - 8/16/32/64 bits registers;
    - GPIO controllers with clear/set registers;
    - GPIO controllers with a single "data" register;
    - Big endian bits/GPIOs ordering (mostly used on PowerPC).

And there are a users of this "basic-mmio-gpio" interface:
arch/arm/mach-s3c64xx/mach-crag6410.c
arch/arm/mach-omap1/board-ams-delta.c
arch/arm/mach-imx/mach-mx21ads.c
arch/arm/mach-clps711x/board-p720t.c
arch/arm/mach-clps711x/board-autcpu12.c

to that end: The patch series proposed by Mr. Fern?ndez simply added a
device-tree-parser for translating the fdt into a bgpio_pdata platform
structure and use the existing interface with the same "basic-mmio-gpio"
compatible string. So unless steps are taken to remove or rename this
"basic-mmio-gpio" interface there will be patches like this (in your inboxes).
[I know: It's a silly idea to actually touch it, as it'll break compatibility]

> So please frame this binding document in terms of the class of hardware
> you are trying to address. Please provide some introduction to the
> assumptions that you are making about said hardware, such that it's
> obvious when one needs a more specific binding.
> 
> I share the same fears that Rob mentioned in that while this may appear
> simple now, the limitations are unclear, and this effectively prevents
> us from accurately/specifically describing some hardware.
Well, I think this has been trainwrecked enough. But there's a way out:
I would really like to preserve the bgpio_basic_mmio_parse_dt so probably
going to introduce a "wd,mbl-gpio" binding for my own hardware and let 
potential new user be able to reuse the generic dt parser. This will cut
down on the work required to add a device. As they will only need to add
an compatible entry to of_device_id list of the driver like this:
	"brcm,bcm63xx-gpio", bgpio_basic_mmio_parse_dt

Will this work for you Linus, Rob and Mark?

That said, I'm still going through the rest of the post. As some of the
issues are still relevant for the "wd,mbl-gpio" as well.

> Regardless, I've taken the time to review the below, and I have a number
> of comments.
> 
> > +Required properties:
> > +	- reg-names: must contain
> > +		"dat" - data register
> > +		may contain
> > +		"set" - data set register
> > +		"clr" - data clear register
> > +		"dirout" - direction output register
> > +		"dirin" - direction input register
> 
> Below, we mention endianness. Are registers an arbitrary number of bytes
> long? In what unit size are they grouped (e.g. {8,16,32,64}-bit)?
No, sadly they can't be (commit log above). The basic_mmio_gpio
device only supports 8/16/32/64 bits registers and you only can
have one register. 

In a nutshell, the basic_mmio_gpio is a glorified
io(read|write)(8|16|32|64)[be] interface. And that's what makes it appealing.
It won't be long until someone clobbers together gpiohog, gpio-led,
gpio-key-polled, led-triggers definitions and implements a full turing machine
within a fdt just for the fun of it.
 
> What are the expectations for the set/clear/dirin/dirout registers? Are
> they write-one to modify, or write-zero to modify?

There's a paragraph in the driver:
"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."

I'll add that to the binding.

About dirin/dirout. There can only be either be one of the two.
dirin: 1 = GPIO is input, 0 = GPIO is output
dirout: 1 = GPIO is output, 1 = GPIO is input

> For the names, don't bother abbreviating (i.e. use "data", "set",
> "clear").
>
> These names might clash with the real names a specific HW model applies
> to its register sets, which means that this binding is exclusive w.r.t.
> a specific binding for a device (i.e. it cannot be extended with
> device-specific information).
> 
> If we're happy with that, then we must call out the expected limitations
> on the use of this binding, or we end up with the not-so-simple-any-more
> issues Rob mentioned previously. 

Sadly the resource names are part of the original commit. It's not possible
without adding a resource rename step in the dt parser or breaking 
compatibility with existing interface. So, the question would be: which one
do you actually want or better yet: "what can we agree on?".
 
> > +	- 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
> 
> Where the cells encode what? I'm guessing this is <$idx $flags>, but you
> should spell that out explicitly.
Ok. I'll add that.
 
> > +	- gpio-controller: Marks the device node as a gpio controller.
> > +
> > +Optional properties:
> > +	- ngpio: specifies the number of gpio mapped in the register.
> > +	- big-endian: force big endian register accesses.
> > +	- big-endian-byte-order: assign GPIOs in reverse order.
> 
> I cannot parse this last description. It needs a better wording.
> 
> I guess this means that the indices (in the first GPIO cell) are applied
> in descending order for ascending chunks of the registers (and that is a
> poor description, too).

I looked up the proper CS terms: 
"big-endian-byte-order: assign GPIOs in MSB 0 bit numbering order."
(The default is: LSB 0 bit numbering)

> > +	- unreadable-reg-set: data set register is not readable.
> > +	- read-output-reg-set: cache value set for reads.
> > +	- unreadable-reg-dir: dirout/dirin register is not readable.
> > +	- no-output: GPIOs are read-only.
> > +
> > +The GPIO generic library provides support for memory-mapped GPIO
> > +controllers. The configuration is detected by which resources are present.
> > +The simplest form of a GPIO controller that the driver support is just a
> > +single "dat" register, where GPIO state can be read and/or written.
> > +However, the driver supports far more:
> > +	- 8/16/32/64 bits registers. The number of GPIOs is automatically
> > +	  determined by the width of the registers.
> > +	- GPIO controllers with clear/set registers.
> > +	- GPIO controllers with a single "dat" register.
> > +	- Big endian bits/GPIOs ordering.
> 
> Reword this in terms of the class of hardware you are trying to support,
> (rather than the specific library code you are using), and move it to
> the introduction at the top of the binding.

Yes. That said the MyBook Live only has a bidirectional "dat" register
and 8 bit for GPIOs (this simplifies a lot of things).

Unless of course, we can somehow agree on the generic approach.
 
Regards,
Christian

[0] <http://marc.info/?l=git-commits-head&m=128867795112158&w=2>

Re: [RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Christian Lamparter]

Hello,

(I'm sort of answering both of yours and Linus' questions)

On Friday, April 29, 2016 12:15:01 PM Mark Rutland wrote:
> As a general thing, please put the binding earlier in a series than code
> implemeting it, as that that makes it easier to review the series
> in-order (with context from the binding making code review easier). See
> Documentation/devicetree/bindings/submitting-patches.txt

Understood. I have to rebase my series anyway since Linus Walleij already
merged two related patches (the bugfix and the rename patch). For the new
series I'll put the binding on the top.
 
> On Fri, Apr 29, 2016 at 02:53:17AM +0200, Christian Lamparter wrote:
> > From: Álvaro Fernández Rojas <noltari@gmail.com>
> > 
> > This patch adds the device tree bindings for the gpio-mmio.
> > The gpio-mmio is already part of a the GPIO generic library.
> > 
> > Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> > ---
> >  .../devicetree/bindings/gpio/gpio-mmio.txt         | 73 ++++++++++++++++++++++
> >  1 file changed, 73 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/gpio/gpio-mmio.txt b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > new file mode 100644
> > index 0000000..cc7f0b1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-mmio.txt
> > @@ -0,0 +1,73 @@
> > +Bindings for the generic driver for memory-mapped GPIO controllers.
> > +
> 
> Bindings should be for hardware (either specific device models, or for
> classes), and not for Linux drivers. The latter is subject to arbitrary
> changes while the former is not, as old hardware continues to exist and
> does not change while drivers get completely reworked.
Ok, maybe here's part of the problem. I'm thinking that the GPIOs devices
I'm referring to:
	MyBook Live
	brcm63xx (which is a popular router SoC so there are many boards)

are the same common class: "memory-mapped gpio". It's is a bit
unfortunate that the linux driver file name was gpio-generic.c. (But
this since been fixed... actually funny thing is that it started as:
basic_mmio_gpio.c)

The important bit is that, when I looked around: there is was already
an existing device binding in the form of a platform_device called:
"basic-mmio-gpio" [0].

commit aeec56e331c6d2750de02ef34b305338305ca690
Author: Anton Vorontsov <cbouatmailru@gmail.com>
Date:   Wed Oct 27 15:33:15 2010 -0700

    gpio: add driver for basic memory-mapped GPIO controllers
    
    The basic GPIO controllers may be 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.
    
    These controllers may not provide any means of pin setup
    (in/out/open drain).
    
    The driver supports:
    - 8/16/32/64 bits registers;
    - GPIO controllers with clear/set registers;
    - GPIO controllers with a single "data" register;
    - Big endian bits/GPIOs ordering (mostly used on PowerPC).

And there are a users of this "basic-mmio-gpio" interface:
arch/arm/mach-s3c64xx/mach-crag6410.c
arch/arm/mach-omap1/board-ams-delta.c
arch/arm/mach-imx/mach-mx21ads.c
arch/arm/mach-clps711x/board-p720t.c
arch/arm/mach-clps711x/board-autcpu12.c

to that end: The patch series proposed by Mr. Fernández simply added a
device-tree-parser for translating the fdt into a bgpio_pdata platform
structure and use the existing interface with the same "basic-mmio-gpio"
compatible string. So unless steps are taken to remove or rename this
"basic-mmio-gpio" interface there will be patches like this (in your inboxes).
[I know: It's a silly idea to actually touch it, as it'll break compatibility]

> So please frame this binding document in terms of the class of hardware
> you are trying to address. Please provide some introduction to the
> assumptions that you are making about said hardware, such that it's
> obvious when one needs a more specific binding.
> 
> I share the same fears that Rob mentioned in that while this may appear
> simple now, the limitations are unclear, and this effectively prevents
> us from accurately/specifically describing some hardware.
Well, I think this has been trainwrecked enough. But there's a way out:
I would really like to preserve the bgpio_basic_mmio_parse_dt so probably
going to introduce a "wd,mbl-gpio" binding for my own hardware and let 
potential new user be able to reuse the generic dt parser. This will cut
down on the work required to add a device. As they will only need to add
an compatible entry to of_device_id list of the driver like this:
	"brcm,bcm63xx-gpio", bgpio_basic_mmio_parse_dt

Will this work for you Linus, Rob and Mark?

That said, I'm still going through the rest of the post. As some of the
issues are still relevant for the "wd,mbl-gpio" as well.

> Regardless, I've taken the time to review the below, and I have a number
> of comments.
> 
> > +Required properties:
> > +	- reg-names: must contain
> > +		"dat" - data register
> > +		may contain
> > +		"set" - data set register
> > +		"clr" - data clear register
> > +		"dirout" - direction output register
> > +		"dirin" - direction input register
> 
> Below, we mention endianness. Are registers an arbitrary number of bytes
> long? In what unit size are they grouped (e.g. {8,16,32,64}-bit)?
No, sadly they can't be (commit log above). The basic_mmio_gpio
device only supports 8/16/32/64 bits registers and you only can
have one register. 

In a nutshell, the basic_mmio_gpio is a glorified
io(read|write)(8|16|32|64)[be] interface. And that's what makes it appealing.
It won't be long until someone clobbers together gpiohog, gpio-led,
gpio-key-polled, led-triggers definitions and implements a full turing machine
within a fdt just for the fun of it.
 
> What are the expectations for the set/clear/dirin/dirout registers? Are
> they write-one to modify, or write-zero to modify?

There's a paragraph in the driver:
"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."

I'll add that to the binding.

About dirin/dirout. There can only be either be one of the two.
dirin: 1 = GPIO is input, 0 = GPIO is output
dirout: 1 = GPIO is output, 1 = GPIO is input

> For the names, don't bother abbreviating (i.e. use "data", "set",
> "clear").
>
> These names might clash with the real names a specific HW model applies
> to its register sets, which means that this binding is exclusive w.r.t.
> a specific binding for a device (i.e. it cannot be extended with
> device-specific information).
> 
> If we're happy with that, then we must call out the expected limitations
> on the use of this binding, or we end up with the not-so-simple-any-more
> issues Rob mentioned previously. 

Sadly the resource names are part of the original commit. It's not possible
without adding a resource rename step in the dt parser or breaking 
compatibility with existing interface. So, the question would be: which one
do you actually want or better yet: "what can we agree on?".
 
> > +	- 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
> 
> Where the cells encode what? I'm guessing this is <$idx $flags>, but you
> should spell that out explicitly.
Ok. I'll add that.
 
> > +	- gpio-controller: Marks the device node as a gpio controller.
> > +
> > +Optional properties:
> > +	- ngpio: specifies the number of gpio mapped in the register.
> > +	- big-endian: force big endian register accesses.
> > +	- big-endian-byte-order: assign GPIOs in reverse order.
> 
> I cannot parse this last description. It needs a better wording.
> 
> I guess this means that the indices (in the first GPIO cell) are applied
> in descending order for ascending chunks of the registers (and that is a
> poor description, too).

I looked up the proper CS terms: 
"big-endian-byte-order: assign GPIOs in MSB 0 bit numbering order."
(The default is: LSB 0 bit numbering)

> > +	- unreadable-reg-set: data set register is not readable.
> > +	- read-output-reg-set: cache value set for reads.
> > +	- unreadable-reg-dir: dirout/dirin register is not readable.
> > +	- no-output: GPIOs are read-only.
> > +
> > +The GPIO generic library provides support for memory-mapped GPIO
> > +controllers. The configuration is detected by which resources are present.
> > +The simplest form of a GPIO controller that the driver support is just a
> > +single "dat" register, where GPIO state can be read and/or written.
> > +However, the driver supports far more:
> > +	- 8/16/32/64 bits registers. The number of GPIOs is automatically
> > +	  determined by the width of the registers.
> > +	- GPIO controllers with clear/set registers.
> > +	- GPIO controllers with a single "dat" register.
> > +	- Big endian bits/GPIOs ordering.
> 
> Reword this in terms of the class of hardware you are trying to support,
> (rather than the specific library code you are using), and move it to
> the introduction at the top of the binding.

Yes. That said the MyBook Live only has a bidirectional "dat" register
and 8 bit for GPIOs (this simplifies a lot of things).

Unless of course, we can somehow agree on the generic approach.
 
Regards,
Christian

[0] <http://marc.info/?l=git-commits-head&m=128867795112158&w=2>

Re: [RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> From: Álvaro Fernández Rojas <noltari@gmail.com>
>
> This patch adds the device tree bindings for the gpio-mmio.
> The gpio-mmio is already part of a the GPIO generic library.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

I share your ambition to create something generic for this class
of hardware(s).

> +Bindings for the generic driver for memory-mapped GPIO controllers.
> +
> +Required properties:
> +       - compatible: should be "linux,gpio-mmio"

Why?
"memory-mapped-gpio" sits nicely with me.

Read another very generic binding for inspiration:
Documentation/devicetree/bindings/leds/register-bit-led.txt

> +       - reg-names: must contain
> +               "dat" - data register
> +               may contain
> +               "set" - data set register
> +               "clr" - data clear register
> +               "dirout" - direction output register
> +               "dirin" - direction input register

I would just be more verbose:

data-in-register
data-out-set-register
data-out-clear-register
direction-output-register
direction-input-register

Some should be optional so we can support input-only
and output-only GPIO controllers too.

I would take this opportunity to add bindings also for stuff that
the generic MMIO driver does not support today but
could be made to support:

open-drain-register
open-source-register
debounce-register

etc

> +Optional properties:
> +       - ngpio: specifies the number of gpio mapped in the register.

Just reference the generic docs.

> +       - big-endian: force big endian register accesses.
> +       - big-endian-byte-order: assign GPIOs in reverse order.
> +       - unreadable-reg-set: data set register is not readable.
> +       - read-output-reg-set: cache value set for reads.
> +       - unreadable-reg-dir: dirout/dirin register is not readable.
> +       - no-output: GPIOs are read-only.

I think it's better to imply that if there is no data-in-register
specified, then it is output-only etc.

> +The GPIO generic library provides support for memory-mapped GPIO
> +controllers. The configuration is detected by which resources are present.
> +The simplest form of a GPIO controller that the driver support is just a
> +single "dat" register, where GPIO state can be read and/or written.
> +However, the driver supports far more:
> +       - 8/16/32/64 bits registers. The number of GPIOs is automatically
> +         determined by the width of the registers.
> +       - GPIO controllers with clear/set registers.
> +       - GPIO controllers with a single "dat" register.
> +       - Big endian bits/GPIOs ordering.


Skip this. It is Linux-specific.

Yours,
Linus Walleij
--
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

[RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> From: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
>
> This patch adds the device tree bindings for the gpio-mmio.
> The gpio-mmio is already part of a the GPIO generic library.
>
> Signed-off-by: ?lvaro Fern?ndez Rojas <noltari@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

I share your ambition to create something generic for this class
of hardware(s).

> +Bindings for the generic driver for memory-mapped GPIO controllers.
> +
> +Required properties:
> +       - compatible: should be "linux,gpio-mmio"

Why?
"memory-mapped-gpio" sits nicely with me.

Read another very generic binding for inspiration:
Documentation/devicetree/bindings/leds/register-bit-led.txt

> +       - reg-names: must contain
> +               "dat" - data register
> +               may contain
> +               "set" - data set register
> +               "clr" - data clear register
> +               "dirout" - direction output register
> +               "dirin" - direction input register

I would just be more verbose:

data-in-register
data-out-set-register
data-out-clear-register
direction-output-register
direction-input-register

Some should be optional so we can support input-only
and output-only GPIO controllers too.

I would take this opportunity to add bindings also for stuff that
the generic MMIO driver does not support today but
could be made to support:

open-drain-register
open-source-register
debounce-register

etc

> +Optional properties:
> +       - ngpio: specifies the number of gpio mapped in the register.

Just reference the generic docs.

> +       - big-endian: force big endian register accesses.
> +       - big-endian-byte-order: assign GPIOs in reverse order.
> +       - unreadable-reg-set: data set register is not readable.
> +       - read-output-reg-set: cache value set for reads.
> +       - unreadable-reg-dir: dirout/dirin register is not readable.
> +       - no-output: GPIOs are read-only.

I think it's better to imply that if there is no data-in-register
specified, then it is output-only etc.

> +The GPIO generic library provides support for memory-mapped GPIO
> +controllers. The configuration is detected by which resources are present.
> +The simplest form of a GPIO controller that the driver support is just a
> +single "dat" register, where GPIO state can be read and/or written.
> +However, the driver supports far more:
> +       - 8/16/32/64 bits registers. The number of GPIOs is automatically
> +         determined by the width of the registers.
> +       - GPIO controllers with clear/set registers.
> +       - GPIO controllers with a single "dat" register.
> +       - Big endian bits/GPIOs ordering.


Skip this. It is Linux-specific.

Yours,
Linus Walleij

Re: [RFC v5 4/4] gpio: dt-bindings: add gpio-mmio bindings

[Linus Walleij]

On Fri, Apr 29, 2016 at 2:53 AM, Christian Lamparter
<chunkeey@googlemail.com> wrote:

> From: Álvaro Fernández Rojas <noltari@gmail.com>
>
> This patch adds the device tree bindings for the gpio-mmio.
> The gpio-mmio is already part of a the GPIO generic library.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>

I share your ambition to create something generic for this class
of hardware(s).

> +Bindings for the generic driver for memory-mapped GPIO controllers.
> +
> +Required properties:
> +       - compatible: should be "linux,gpio-mmio"

Why?
"memory-mapped-gpio" sits nicely with me.

Read another very generic binding for inspiration:
Documentation/devicetree/bindings/leds/register-bit-led.txt

> +       - reg-names: must contain
> +               "dat" - data register
> +               may contain
> +               "set" - data set register
> +               "clr" - data clear register
> +               "dirout" - direction output register
> +               "dirin" - direction input register

I would just be more verbose:

data-in-register
data-out-set-register
data-out-clear-register
direction-output-register
direction-input-register

Some should be optional so we can support input-only
and output-only GPIO controllers too.

I would take this opportunity to add bindings also for stuff that
the generic MMIO driver does not support today but
could be made to support:

open-drain-register
open-source-register
debounce-register

etc

> +Optional properties:
> +       - ngpio: specifies the number of gpio mapped in the register.

Just reference the generic docs.

> +       - big-endian: force big endian register accesses.
> +       - big-endian-byte-order: assign GPIOs in reverse order.
> +       - unreadable-reg-set: data set register is not readable.
> +       - read-output-reg-set: cache value set for reads.
> +       - unreadable-reg-dir: dirout/dirin register is not readable.
> +       - no-output: GPIOs are read-only.

I think it's better to imply that if there is no data-in-register
specified, then it is output-only etc.

> +The GPIO generic library provides support for memory-mapped GPIO
> +controllers. The configuration is detected by which resources are present.
> +The simplest form of a GPIO controller that the driver support is just a
> +single "dat" register, where GPIO state can be read and/or written.
> +However, the driver supports far more:
> +       - 8/16/32/64 bits registers. The number of GPIOs is automatically
> +         determined by the width of the registers.
> +       - GPIO controllers with clear/set registers.
> +       - GPIO controllers with a single "dat" register.
> +       - Big endian bits/GPIOs ordering.


Skip this. It is Linux-specific.

Yours,
Linus Walleij